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] Materialize immediates with 64-bit ORR + EOR if shorter #68287

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

dougallj
Copy link
Contributor

@dougallj dougallj commented Oct 5, 2023

A number of useful constants can be encoded with a 64-bit ORR followed by a 64-bit EOR, including all remaining repeated byte patterns, some useful repeated 16-bit patterns, and some irregular masks. This patch prioritizes that encoding over three or four instruction encodings. Encoding with MOV + MOVK or ORR + MOVK is still preferred for fast literal generation and readability respectively.

The method devises three candidate values, and checks if both Candidate and (Imm ^ Candidate) are valid logical immediates. If so, Imm is materialized with:

ORR Xd, XZR, #(Imm ^ Candidate)
EOR Xd, Xd, #(Candidate)

The method has been exhaustively tested to ensure it can solve all possible values (excluding 0, ~0, and plain logical immediates, which are handled earlier).

@github-actions
Copy link

github-actions bot commented Oct 5, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@dougallj dougallj force-pushed the aarch64_orr_eor_immediates branch 2 times, most recently from cc5d6d2 to c6d7bee Compare October 6, 2023 04:26
@dougallj
Copy link
Contributor Author

dougallj commented Oct 6, 2023

Contributing to LLVM asks me to select suitable reviewers – I don't think I have permission to do that on Github, but any chance of a review, @davemgreen or @resistor?

@jroelofs jroelofs requested a review from aemerson October 6, 2023 15:50
Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

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

Hello. Sorry for the delay, this looks like a really good patch I just had to try to convince myself how it worked. It LGTM, but I just have a question about one of the details.

llvm/lib/Target/AArch64/AArch64ExpandImm.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/AArch64/AArch64ExpandImm.cpp Outdated Show resolved Hide resolved
llvm/lib/Target/AArch64/AArch64ExpandImm.cpp Show resolved Hide resolved
A number of useful constants can be encoded with a 64-bit ORR followed
by a 64-bit EOR, including all remaining repeated byte patterns, some
useful repeated 16-bit patterns, and some irregular masks. This patch
prioritizes that encoding over three or four instruction encodings.
Encoding with MOV + MOVK or ORR + MOVK is still preferred for fast
literal generation and readability respectively.

The method devises three candidate values, and checks if both Candidate
and (Imm ^ Candidate) are valid logical immediates. If so, Imm is
materialized with:

```
ORR Xd, XZR, #(Imm ^ Candidate)
EOR Xd, Xd, #(Candidate)
```

The method has been exhaustively tested to ensure it can solve all
possible values (excluding 0, ~0, and plain logical immediates, which
are handled earlier).
@dougallj dougallj force-pushed the aarch64_orr_eor_immediates branch from c6d7bee to 1ff2eb1 Compare October 8, 2023 12:34
Copy link
Collaborator

@davemgreen davemgreen left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM

Are you able to hit submit, or do I need to do that? If so are you happy for this to go in?

@dougallj
Copy link
Contributor Author

Thanks for the review! I'm not able to hit submit, but I am happy for this to be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants