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

aarch64: fix reg/imm sub insts that read SP, not the zero register. #2548

Merged
merged 1 commit into from
Jan 6, 2021

Conversation

cfallin
Copy link
Member

@cfallin cfallin commented Jan 5, 2021

On AArch64, the zero register (xzr) and the stack pointer (xsp) are
alternately named by the same index 31 in machine code depending on
context. In particular, in the reg-reg-immediate ALU instruction form,
add/subtract will use the stack pointer, not the zero register, if index
31 is given for the first (register) source arg.

In a few places, we were emitting subtract instructions with the zero
register as an argument and a reg/immediate as the second argument. When
an immediate could be incorporated directly (we have the iconst
definition visible), this would result in incorrect code being
generated.

This issue was found in ineg and in the sequence for vector
right-shifts.

Reported by Ian Cullinan; thanks!

On AArch64, the zero register (xzr) and the stack pointer (xsp) are
alternately named by the same index `31` in machine code depending on
context. In particular, in the reg-reg-immediate ALU instruction form,
add/subtract will use the stack pointer, not the zero register, if index
31 is given for the first (register) source arg.

In a few places, we were emitting subtract instructions with the zero
register as an argument and a reg/immediate as the second argument. When
an immediate could be incorporated directly (we have the `iconst`
definition visible), this would result in incorrect code being
generated.

This issue was found in `ineg` and in the sequence for vector
right-shifts.

Reported by Ian Cullinan; thanks!
@cfallin cfallin requested a review from fitzgen January 5, 2021 23:58
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

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

Looks great!

@fitzgen fitzgen merged commit 6317290 into bytecodealliance:main Jan 6, 2021
@cfallin cfallin deleted the fix-aarch64-sp branch January 6, 2021 18:00
julian-seward1 pushed a commit to mozilla-spidermonkey/wasmtime that referenced this pull request Jan 14, 2021
On AArch64, the zero register (xzr) and the stack pointer (xsp) are
alternately named by the same index `31` in machine code depending on
context. In particular, in the reg-reg-immediate ALU instruction form,
add/subtract will use the stack pointer, not the zero register, if index
31 is given for the first (register) source arg.

In a few places, we were emitting subtract instructions with the zero
register as an argument and a reg/immediate as the second argument. When
an immediate could be incorporated directly (we have the `iconst`
definition visible), this would result in incorrect code being
generated.

This issue was found in `ineg` and in the sequence for vector
right-shifts.

Reported by Ian Cullinan; thanks!

(cherry picked from commit aac3751)
(this is bytecodealliance#2548)
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.

2 participants