Stream: git-wasmtime

Topic: wasmtime / issue #7121 riscv64: Refactor and enable optim...


view this post on Zulip 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:

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

view this post on Zulip 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

view this post on Zulip 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 Value to 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 uextend and sextend because 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 via sext and zext. 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. an addw equivalent) 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 *w instructions are simple enough to RISC-V it may be good enough to simply have all the lowerings in sext and zext and 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.

view this post on Zulip 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.

view this post on Zulip 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 i128 in the various backends and building more complete support is a lot of work. Additionally most of them are fairly similar (idiv.i128 is 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: Dec 23 2024 at 12:05 UTC