alexcrichton opened PR #5905 from shuffle-cases
to main
:
I noticed this difference between LLVM and Cranelift for something I was looking at recently, and while it's probably not all that common I figured I'd add it here since it should be somewhat useful nevertheless.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
jameysharp submitted PR review.
jameysharp submitted PR review.
jameysharp created PR review comment:
This comment should say "lower lanes", right?
jameysharp created PR review comment:
I think the hex literals in the filetest actually make the pattern more clear here. What would you think of replacing these decimal literals with hex?
Optionally, I suppose
vec_mask_is
could take au128
instead of 16u8
s. I like the idea of not having a bazillion arguments to that function, but I'm not sure whether it's more clear than writing the bytes out separately. What do you think?
alexcrichton updated PR #5905 from shuffle-cases
to main
.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Seems reasonable to me!
alexcrichton has enabled auto merge for PR #5905.
jameysharp submitted PR review.
jameysharp created PR review comment:
And now that I'm looking at that revision, it occurs to me that if you make a
u128_from_vec_mask
extractor instead ofvec_mask_is
, you can put these two rules at the same priority because ISLE will be able to tell that their different masks don't overlap. That should generate better pattern-matching code. Not super important, but would be nice.
alexcrichton updated PR #5905 from shuffle-cases
to main
.
alexcrichton submitted PR review.
alexcrichton created PR review comment:
Oh good point, and that removes the need for
vec_mask_is
sinceu128_from_immediate
already exists too
alexcrichton has enabled auto merge for PR #5905.
alexcrichton merged PR #5905.
Last updated: Nov 22 2024 at 17:03 UTC