jameysharp opened PR #5746 from opts-shifts
to main
.
jameysharp requested cfallin for a review on PR #5746.
jameysharp submitted PR review.
jameysharp created PR review comment:
I'm a little confused about the requirements for
iconst
. Its documentation says it can create "an integer vector where all the lanes have the same value", which probably should work here. Do I just pass the lane type instead of the originalty
in some of these places?ty_mask
, at least, and also maybeimm64_ushr
I guess?
cfallin submitted PR review.
cfallin created PR review comment:
Hmm -- that feature is somewhat surprising to me! I don't think we actually have lowerings that do that (and in fact I think we have another opcode for "vector broadcast" or whatever the proper name for that operation is)?
bjorn3 created PR review comment:
and in fact I think we have another opcode for "vector broadcast" or whatever the proper name for that operation is
It's called
splat
.
bjorn3 submitted PR review.
cfallin submitted PR review.
cfallin submitted PR review.
cfallin created PR review comment:
(same here as above)
cfallin created PR review comment:
(After talking with Jamey offline about this) it seems this is actually an equality constraint, counterintuitively! The
narrow_mask
is already defined and we start with the existing binding environment, not a fresh one, when building the AST for the LHS pattern; so we get the behavior one would for a second use of a variable in a pattern.Here we should write an explicit equality (e.g.
(if-let $true (u64_eq ...))
) I think, and separately think about the above binding behavior...
cfallin created PR review comment:
This would be a little easier to parse if it were split across multiple lines, IMHO (just a preference!).
cfallin created PR review comment:
(same comment here re: equality test)
cfallin created PR review comment:
(same comment re: equality)
cfallin created PR review comment:
Just for clarity, even though it's strictly redundant given CLIF typechecking, could we write
wide
for the wildcards for the type params toishl
andsextend
here? (General comment across the rules, and we may not have been consistent elsewhere with this; but especially for rules operating across multiple types, it makes it much easier to reason about!)
jameysharp updated PR #5746 from opts-shifts
to main
.
jameysharp updated PR #5746 from opts-shifts
to main
.
jameysharp updated PR #5746 from opts-shifts
to main
.
cfallin submitted PR review.
jameysharp merged PR #5746.
Last updated: Nov 22 2024 at 17:03 UTC