Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

i#5365 AArch64 SVE core, part 2: add signals support #6725

Merged
merged 28 commits into from
Apr 3, 2024

Conversation

AssadHashmi
Copy link
Contributor

This patch adds SVE support for signals in the core. It is the follow on patch from the SVE core work part 1, in PR #5835 (f646a63) and includes vector address computation for SVE scatter/gather, enabling first-fault load handling.

Issue: #5365, #5036

Co-authored-by: Jack Gallagher [email protected]

AssadHashmi and others added 3 commits March 26, 2024 11:36
This patch adds SVE support for signals in the core. It is the follow
on patch from the SVE core work part 1, in PR #5835 (f646a63) and
includes vector address computation for SVE scatter/gather, enabling
first-fault load handling.

Issue: #5365, #5036

Co-authored-by: Jack Gallagher <[email protected]>
Copy link
Contributor

@derekbruening derekbruening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe there should be another round just due to the # of comments.

core/arch/arch.h Show resolved Hide resolved
core/ir/opnd.h Outdated Show resolved Hide resolved
core/ir/aarch64/instr.c Outdated Show resolved Hide resolved
core/ir/aarch64/instr.c Outdated Show resolved Hide resolved
core/ir/aarch64/instr.c Outdated Show resolved Hide resolved
suite/runsuite_wrapper.pl Outdated Show resolved Hide resolved
suite/tests/api/opnd-a64.c Outdated Show resolved Hide resolved
suite/tests/api/opnd-a64.c Outdated Show resolved Hide resolved
suite/tests/api/opnd-a64.c Show resolved Hide resolved
suite/tests/tools.c Show resolved Hide resolved
@AssadHashmi AssadHashmi changed the title i#5365: Add AArch64 SVE support to the core (part 2) i#5365 AArch64 SVE core, part 2: add signals support Apr 1, 2024
@AssadHashmi
Copy link
Contributor Author

Maybe there should be another round just due to the # of comments.

Re-requested review.

core/ir/opnd_api.h Outdated Show resolved Hide resolved
core/unix/signal_linux_aarch64.c Show resolved Hide resolved
core/unix/signal_linux_aarch64.c Outdated Show resolved Hide resolved
core/ir/opnd_shared.c Show resolved Hide resolved
AssadHashmi and others added 5 commits April 3, 2024 09:59
…er addressing modes.

Add reference to i#6750 in implementation of reg_is_pointer_sized().
Explicitly remind readers that quadwords in AArch64 are 128 bits.
Clarify vector doubleword destination in explanatory comments
A mistake in the original patch flipped the position of the #if and if()
lines:

     if (opnd_is_vsib(curop)) {
#ifdef X86

became:

 #if defined(X86) || defined(AARCH64)
     if (opnd_is_vector_base_disp(curop)) {

which meant the CLIENT_ASSERT() which followed it would always be hit on
ARM or RISCV64.

I have changed it to remove the #if defined(..) lines and provide a
stub implementation of instr_compute_vector_address() for AArch32 and
RISC-V.
@AssadHashmi AssadHashmi merged commit 34b7435 into master Apr 3, 2024
15 of 16 checks passed
@AssadHashmi AssadHashmi deleted the i5365-aarch64-sve-veclen-part2-signals branch April 3, 2024 14:09
abhinav92003 added a commit that referenced this pull request Apr 5, 2024
Fixes the slot used to save and restore FP regs at fcache enter and
return events. PR #6725 adjusted the slots used during signal handling
in core/unix/signal_linux_aarch64.c but did not adjust the same in
fcache enter/exit and attach events. Prior to that the FP regs were
simply stored in a continuous manner in signal handling code, and
fcache enter/exit routines.

The mismatch between slot usage caused failueres in the signalNNN1
tests on some systems.
abhinav92003 added a commit that referenced this pull request Apr 5, 2024
Fixes the slot used to save and restore FP regs at fcache enter and
return events. PR #6725 adjusted the slots used during signal handling
in core/unix/signal_linux_aarch64.c but did not adjust the same in
fcache enter/return and attach events. Prior to that PR, the FP regs
were simply stored in a contiguous manner in signal handling code and
fcache enter/return routines (instead of in their respective dr_simd_t
struct member), which was a bit confusing.

The mismatch between slot usage in signal handling and fcache
enter/return code caused failures in the signalNNN1 tests on some
systems. Verified that those tests pass with this fix.

Also fixes the same issue in save_priv_mcontext_helper which is used in
the dr_app_start API. Unit tests for this scenario will be added as part
of #6759.

Issue: #5036, #6755, #5365
Fixes #6758
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants