Wasmtime GitHub notifications bot (Oct 01 2023 at 21:44): Wasmtime GitHub notifications bot (Oct 01 2023 at 21:44):github-actions[bot] commented on issue #7121:
Subscribe to Label Action
cc @cfallin, @fitzgen
<details>
This issue or pull request has been labeled: "cranelift", "isle"Thus the following users have been cc'd because of the following labels:
- cfallin: isle
- fitzgen: isle
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 (Oct 02 2023 at 11:27):afonso360 commented on issue #7121:
It looks like the OpenVINO installer may have an outdated package checksum. cc: @abrown
Wasmtime GitHub notifications bot (Oct 02 2023 at 14:54):alexcrichton commented on issue #7121:
Oh nice, I had totally missed those! Please feel free to do follow-ups, I was going to work on icmp next myself and it seems like you've got a good handle on what needs to fit in here.
This is interesting though because I'm taking inspiration from the x64 backend for this PR where it pattern-matches a
Valueto skip an extension.One point also possibly worth mentioning is that we may not, at this time, be able to put most of the logic here in
uextendandsextendbecause extension is requested by many other instructions due to how the architecture works - this is why I wanted to refactor like this because now the backend can request extension outside of those CLIF instructions viasextandzext. That being said it's still a bit weird I think. At least the way it works on x64 is that during an extension operation no instructions are emitted for the operation being extended (e.g. anaddwequivalent) but instead the choice to omit an extension instruction is made. This implicitly couples the lowering of 32-bit add plus a sign extension for example. This is not great because if the lowering for a 32-bit add changes then it may not be acompanied with an update to the optimization of skipping the sign extension. This has led to some possible optimizations being omitted in the x64 backend for example because the logic is too complicated to ensure they're in sync on "both halves". Basically I think this would be an interesting problem to solve, but as you've pointed out the*winstructions are simple enough to RISC-V it may be good enough to simply have all the lowerings insextandzextand I think that should work out?That would at least work out really well to avoid the problem of having two locations to keep in sync and instead only one has to be looked at.
Wasmtime GitHub notifications bot (Oct 02 2023 at 14:58):alexcrichton commented on issue #7121:
Alternatively I've also been musing lately due to all this if we should perhaps support target-specific rewrites of CLIF. One way to solve some of these issue is to simply remove lowerings that the backend doesn't support. For example RISC-V 64-bit doesn't support 32-bit comparisons (AFAIK at least) so a target-specific rewrite could change all comparisons to being 64-bit comparisons. Other misc examples include how there's no u8-to-f32 instruction on RISC-V so we're required to sign-extend to u32 and then convert. Doing at this at the CLIF layer would be quite nice since it could even benefit from egraph-based optimizations.
Wasmtime GitHub notifications bot (Oct 02 2023 at 19:10):afonso360 commented on issue #7121:
I've had some thoughts about doing something similar but for a slightly different purpose. We have a lot of the unimplemented ops for
i128in the various backends and building more complete support is a lot of work. Additionally most of them are fairly similar (idiv.i128is pretty much always going to be a libcall), so we could have the backend request expanded versions of the ops that they would know how to lower.
Last updated: Nov 22 2024 at 16:03 UTC