x64: Add shuffle cases for punpck{h,l}bw#5905
Conversation
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.
jameysharp
left a comment
jameysharp
left a comment
There was a problem hiding this comment.
Yup, this makes sense to me! I think one comment is wrong, but with that fixed feel free to merge.
| ;; Special case for the `punpckhbw` instruction which interleaves the upper | ||
| ;; lanes of the two input registers. | ||
| (rule 5 (lower (shuffle a b (vec_mask_from_immediate mask))) | ||
| (if-let $true (vec_mask_is mask 8 24 9 25 10 26 11 27 12 28 13 29 14 30 15 31)) |
There was a problem hiding this 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 a u128 instead of 16 u8s. 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?
There was a problem hiding this comment.
Seems reasonable to me!
There was a problem hiding this 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 of vec_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.
There was a problem hiding this comment.
Oh good point, and that removes the need for vec_mask_is since u128_from_immediate already exists too
github-actions
Bot
added
cranelift
Subscribe to Label ActionDetailsThis issue or pull request has been labeled: "cranelift", "cranelift:area:x64", "isle"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
2 participants
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.