github-actions[bot] commented on Issue #1616:
Subscribe to Label Action
cc @bnjbvr
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:meta"Thus the following users have been cc'd because of the following labels:
- bnjbvr: cranelift
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
whitequark commented on Issue #1616:
Is there anything I need to do to get this PR merged? It doesn't depend on anything else.
whitequark commented on Issue #1616:
or I should have reason to be suspicious of this legalization (which otherwise seems entirely fine)
When I wrote that, it was because to me it looked like a legalization splitting
iconcat
s should be in thenarrow
group, but I tried putting it there and determined that it only works in theexpand
group. If it looks good to you, it's probably just me being very new to Cranelift.
whitequark commented on Issue #1616:
Oh, and I also found it odd that it didn't work without the
copy
, like in #1152, but after that I reviewed the Cranelift legalizer and figured that it's just written that way for some reason.
iximeow commented on Issue #1616:
should be in the
narrow
group,I'd only thought about the
narrow
group for narrowing SIMD operations, though I see it's used for quite a bit more. And, that the legalization in fact does not seem to apply when in that group.I found that in
narrow
orwiden
that only one of the legalizations actually made it to the generated legalizer (the i128 reduction) but from the failing test that doesn't seem to apply either. I _think_ what's happening is theireduce
legalization has bounded types, so bylegalize_monomorphic
in the x86_32 cpu definition legalization is run throughexpand_flags
, which eventually is chained to the baseexpand
legalizer. I don't know why codegen would vary across groups though.I see other legalizations through
narrow
appear to have an unbounded parameter, or use a type otherwise undeclared (likeI128
) , so I think this about where to look. And I believeireduce
falls over with an unbounded type because of the constraint that the result must be half the size of the input.This all definitely is not straightforward, and I suspect the missing cases when these legalizations are in
narrow
orwiden
might be a bug in the code generator for the legalizer, but finding a somewhat compelling reason why this ought to be inexpand
is all I was really after. Since this is all slated for :boom: I'll stop here :)
Last updated: Dec 23 2024 at 12:05 UTC