Stream: git-wasmtime

Topic: wasmtime / Issue #1616 Legalize ireduce.iN.i2N to isplit

Wasmtime GitHub notifications bot (Apr 28 2020 at 11:10):

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>

Wasmtime GitHub notifications bot (May 02 2020 at 03:51):

whitequark commented on Issue #1616:

Is there anything I need to do to get this PR merged? It doesn't depend on anything else.

Wasmtime GitHub notifications bot (May 05 2020 at 21:16):

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 iconcats should be in the narrow group, but I tried putting it there and determined that it only works in the expand group. If it looks good to you, it's probably just me being very new to Cranelift.

Wasmtime GitHub notifications bot (May 05 2020 at 21:18):

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.

Wasmtime GitHub notifications bot (May 06 2020 at 01:32):

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 or widen 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 the ireduce legalization has bounded types, so by legalize_monomorphic in the x86_32 cpu definition legalization is run through expand_flags, which eventually is chained to the base expand 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 (like I128) , so I think this about where to look. And I believe ireduce 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 or widen might be a bug in the code generator for the legalizer, but finding a somewhat compelling reason why this ought to be in expand is all I was really after. Since this is all slated for :boom: I'll stop here :)

Last updated: Nov 22 2024 at 17:03 UTC