Skip to content

Commit

Permalink
Addressed final set of review comments:
Browse files Browse the repository at this point in the history
- Updated release notes to include compatibility-affecting changes due
  to dr_mcontext_t and dr_simd_t support for SVE.
- Added binary compatibility machine context size check in
  priv_mcontext_to_dr_mcontext() for DR_MC_MULTIMEDIA clients in the
  case of pre-SVE clients.
- Made AArch32 SIMD registers comment private (non-Doxygen).
- Updated ignore list in runsuite_wrapper.pl for QEMU based regressions,
  issue #6260.
  • Loading branch information
AssadHashmi committed Aug 10, 2023
1 parent 3c03659 commit a57fb68
Show file tree
Hide file tree
Showing 4 changed files with 30 additions and 33 deletions.
5 changes: 5 additions & 0 deletions api/docs/release.dox
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,11 @@ Further non-compatibility-affecting changes include:
- Added proc_get_vector_length_bytes() for AArch64. This returns the current
vector length on all ARMv8 hardware including hardware which supports the
Scalable Vector Extension (SVE).
- The addition of AArch64 SVE support required changes to dr_mcontext_t and
dr_simd_t. In addition to the vector registers simd[] in dr_mcontext_t,
there are also SVE predicate registers svep[] and the SVE first-fault
register, ffr. The dr_simd_t register storage has increased in size from 128
bits to 512.

**************************************************
<hr>
Expand Down
37 changes: 9 additions & 28 deletions core/arch/arch.c
Original file line number Diff line number Diff line change
Expand Up @@ -3550,6 +3550,14 @@ priv_mcontext_to_dr_mcontext(dr_mcontext_t *dst, priv_mcontext_t *src)
*/
if (dst->size > sizeof(dr_mcontext_t))
return false;
#if defined(AARCH64)
/* Clients built before support of Arm AArch64's Scalable Vector Extension
* (SVE) are not binary compatible with the latest build.
*/
if (TEST(DR_MC_MULTIMEDIA, dst->flags) && dst->size != sizeof(dr_mcontext_t))
CLIENT_ASSERT(
false, "A pre-SVE client is running on an Arm AArch64 SVE DynamoRIO build!");
#endif
if (TESTALL(DR_MC_ALL, dst->flags) && dst->size == sizeof(dr_mcontext_t)) {
*(priv_mcontext_t *)(&MCXT_FIRST_REG_FIELD(dst)) = *src;
} else {
Expand Down Expand Up @@ -3634,34 +3642,7 @@ priv_mcontext_to_dr_mcontext(dr_mcontext_t *dst, priv_mcontext_t *src)
memcpy(&dst->opmask, &src->opmask, sizeof(dst->opmask));
}
#elif defined(AARCHXX)
# ifdef X64
/* The first and so far (August 2023) only DR_MC_MULTIMEDIA related
* change to AARCH64's machine context was addition of the Scalable
* Vector Extension (SVE) vector registers (DR_REG_Z0->DR_REG_Z31),
* predicate registers (DR_REG_P0->DR_REG_P15) and first fault
* register (DR_REG_FFR). Further SVE2 support did not change
* these. To avoid breaking backward compatibility with pre-SVE
* clients, we only copy the first 128 bits of each vector, i.e.
* the v8 NEON SIMD registers DR_REG_Q0->DR_REG_Q31.
*/
if (sizeof(dst->simd) < sizeof(src->simd)) {
/* For pre-SVE and SVE builds:
* MCXT_NUM_SIMD_SLOTS = MCXT_NUM_SIMD_SVE_SLOTS = 32.
* with the only difference in simd[] being the size of each
* simd slot.
*/
for (int i = 0; i < MCXT_NUM_SIMD_SVE_SLOTS; ++i)
memcpy(&dst->simd[i], &src->simd[i], 16);
} else if (sizeof(dst->simd) > sizeof(src->simd))
CLIENT_ASSERT(false, "An SVE client is running on a pre-SVE DR build!");
else
return false;
# else
/* TODO i#1551: NYI on AARCH32. */
ASSERT_NOT_IMPLEMENTED(false);
# endif
#else
/* TODO i#3544: NYI on RISC-V. */
/* FIXME i#1551: NYI on ARM */
ASSERT_NOT_IMPLEMENTED(false);
#endif
}
Expand Down
13 changes: 8 additions & 5 deletions core/lib/mcxtx_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,15 @@
*/
dr_simd_t ffr;
# else
/*
* For the Arm AArch32 SIMD registers, we would probably be ok if we did
* not preserve the callee-saved registers (q4-q7 == d8-d15) but to be safe
* we preserve them all. We do not need anything more than word alignment
* for OP_vldm/OP_vstm, and dr_simd_t has no fields larger than 32 bits, so
* we have no padding.
*/
/**
* The Arm AArch32 SIMD registers. We would probably be ok if we did not
* preserve the callee-saved registers (q4-q7 == d8-d15) but to be safe we
* preserve them all. We do not need anything more than word alignment for
* OP_vldm/OP_vstm, and dr_simd_t has no fields larger than 32 bits, so we
* have no padding.
* The Arm AArch32 SIMD registers.
*/
dr_simd_t simd[MCXT_NUM_SIMD_SLOTS];
# endif
Expand Down
8 changes: 8 additions & 0 deletions suite/runsuite_wrapper.pl
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
my $mydir = dirname(abs_path($0));
my $is_CI = 0;
my $is_aarchxx = $Config{archname} =~ /(aarch64)|(arm)/;
my $is_x86_64 = $Config{archname} =~ /x86_64/;
my $is_long = $ENV{'CI_TRIGGER'} eq 'push' && $ENV{'CI_BRANCH'} eq 'refs/heads/master';

# Forward args to runsuite.cmake:
Expand Down Expand Up @@ -348,6 +349,13 @@
} else {
$issue_no = "#2417";
}
} elsif ($is_x86_64 && ($ENV{'DYNAMORIO_CROSS_AARCHXX_LINUX_ONLY'} eq 'yes') && $args =~ /64_only/) {
# These AArch64 cross-compiled tests fail on x86-64 QEMU but pass
# on native AArch64 hardware.
$ignore_failures_64{'code_api|client.drx_buf-test'} = 1;
$ignore_failures_64{'code_api|sample.memval_simple'} = 1;
$ignore_failures_64{'code_api|client.drreg-test'} = 1;
$issue_no = "#6260";
} elsif ($^O eq 'darwin') {
%ignore_failures_32 = ('code_api|common.decode-bad' => 1, # i#3127
'code_api|linux.signal0000' => 1, # i#3127
Expand Down

0 comments on commit a57fb68

Please sign in to comment.