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

asm: add more ALU instructions, fix sign-extended immediates #10214

Closed
wants to merge 5 commits into from

Conversation

abrown
Copy link
Contributor

@abrown abrown commented Feb 10, 2025

This set of changes adds several more classes of ALU instructions (add, adc, sub, sbb, or, xor). It also resolves #10200 by adding new types for sign-extended immediates. More details in each commit message.

As mentioned in bytecodealliance#10200, `capstone` has a peculiar way of pretty-printing
immediates, especially signed immediates. It is simpler (and perhaps
more clear) for us to just print immediates in one consistent format:
`0xffff...`, e.g. This change parses capstone's pretty-printed
immediates and converts them to our simpler format if the first attempt
to match this assembler's output with `capston` fails.
As pointed out in bytecodealliance#10200, it could be confusing for users for
`cranelift-assembler-x64` to pass unsigned integers to certain assembler
instructions and have them unexpectedly sign-extended. Well, it can't be
too surprising since these instructions have a `_sx*` suffix, but this
change implements @alexcrichton's additional suggestion to create
separate types for the immediates that may be sign-extended.

These new types (`Simm8`, `Simm16`, `Simm32`) are quite similar to their
vanilla counterparts (`Imm8`, `Imm16`, `Imm32`) but have additional
sign-extension logic when pretty-printed. This means the vanilla
versions can be simplified and the pre-existing `Simm32` is renamed to
the more appropriate `AmodeOffset`.
@abrown abrown requested a review from a team as a code owner February 10, 2025 23:04
@abrown abrown requested review from cfallin and removed request for a team February 10, 2025 23:04
@abrown
Copy link
Contributor Author

abrown commented Feb 10, 2025

Now that I think of it, we could split this into two separate PRs for tracking: one to fix #10200 and then a separate one to add the new ALU instructions (especially since I haven't used them in ISLE yet).

@abrown abrown closed this Feb 10, 2025
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.

asm: pretty-printing signed immediates
1 participant