Legalize imul.i64x2 for both AVX and non-AVX x86 CPUs#1759
Merged
abrown merged 9 commits intobytecodealliance:masterfrom Jun 3, 2020
Merged
Legalize imul.i64x2 for both AVX and non-AVX x86 CPUs#1759abrown merged 9 commits intobytecodealliance:masterfrom
abrown merged 9 commits intobytecodealliance:masterfrom
Conversation
e48a82c to
fdcfd1c
Compare
Subscribe to Label Actioncc @bnjbvr DetailsThis issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:x64", "cranelift:meta"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
85cd9eb to
57cc3e2
Compare
bnjbvr
reviewed
Jun 2, 2020
Member
bnjbvr
left a comment
There was a problem hiding this comment.
Looks pretty good! I wonder if the legalization couldn't be done in the meta-language directly using an ISA predicate (and thus avoiding the Any and downcasting).
bnjbvr
approved these changes
Jun 3, 2020
Member
bnjbvr
left a comment
There was a problem hiding this comment.
Unless guarding-legalizations-against-ISA-settings is easy to implement, the proposed approach here works fine. Thanks for the patch!
Without this special instruction, legalizing to the AVX512 instruction AND the SSE instruction sequence is impossible. This extra instruction would be rendered unnecessary by the x64 backend.
This is necessary when we would like to check specific ISA flags, e.g.
This instruction multiplies the lower 32 bits of two 64x2 unsigned integers into an i64x2; this is necessary for lowering Wasm's i64x2.mul.
This instruction does not exist in the SSE2 feature set; it can be added later with an VEX/EVEX encoding.
The `convert_i64x2_imul` custom legalization checks the ISA flags for AVX512DQ or AVX512VL support and legalizes `imul.i64x2` to an `x86_pmullq` in this case; if not, it uses a lengthy SSE2-compatible instruction sequence.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
convert_i64x2_imulcustom legalization checks the ISA flags for AVX512DQ or AVX512VL support and legalizesimul.i64x2to anx86_pmullqin this case; if not, it uses a lengthy SSE2-compatible instruction sequence. For this logic to work, we need:x86_pmullq(this additional instruction would go away in the new backend)TargetIsaso that we can inspect its flags during legalizationx86_pmuludqfor implementing the SSE2-compatible instruction sequence