Wasmtime GitHub notifications bot (May 29 2020 at 18:00): Wasmtime GitHub notifications bot (May 29 2020 at 18:00):github-actions[bot] commented on Issue #1789:
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 29 2020 at 20:03):whitequark commented on Issue #1789:
Hm, I think I still don't fully understand the machinery here, especially around
istore32and*load8...
Wasmtime GitHub notifications bot (Jun 03 2020 at 22:37):abrown commented on Issue #1789:
I think I'm ok merging this if we add some comments like those in https://github.com/bytecodealliance/wasmtime/issues/1747#issuecomment-636148449 about what the controlling type variable is. This isn't really a problem with the PR as much as it is with the types of the instruction definitions being unclear but at least with comments we'll be able to troubleshoot these legalizations more easily. Also, I would have expected to see these legalizations in
widen, notexpandso if it can't be changed there we should probably explain that as well.
Wasmtime GitHub notifications bot (Jun 03 2020 at 22:41):whitequark commented on Issue #1789:
So the problem is that unlike my earlier PRs I actually still don't really understand why it works, and why some of the earlier (incorrect) iterations worked and some of them were broken.
Wasmtime GitHub notifications bot (Jun 04 2020 at 00:14):abrown commented on Issue #1789:
Hm, yeah... Cranelift's legalizations are a bit tricky. Just to confirm the intent of all of this, you want to be able to run
istore32,sload32, anduload32on 32-bit x86, right?
Wasmtime GitHub notifications bot (Jun 04 2020 at 00:16):whitequark commented on Issue #1789:
Yes. Moreover, I have the exact same problem with the
store8/load8variants of instructions, and I assume alsostore16/load16though I haven't encountered those. The problem is that applying the knowledge from this PR to thestore8/load8variants has not been successful as it looks like the meta code requires the cvar to be half the size of the result, whereas they should be independent.
Wasmtime GitHub notifications bot (Jun 04 2020 at 00:31):abrown commented on Issue #1789:
My comments are aimed at clarifying what we want these legalizations to do. But it occurs to me that this may be quicker in IM. I'm usually in https://bytecodealliance.zulipchat.com/#narrow/stream/217117-cranelift and can discuss more tomorrow.
Wasmtime GitHub notifications bot (Jun 04 2020 at 00:32):abrown edited a comment on Issue #1789:
My comments are aimed at clarifying what we want these legalizations to do (so feel free to correct me above if I'm not getting the intent right). But it occurs to me that this may be quicker in IM. I'm usually in https://bytecodealliance.zulipchat.com/#narrow/stream/217117-cranelift and can discuss more tomorrow.
Wasmtime GitHub notifications bot (Jul 05 2020 at 08:50):whitequark commented on Issue #1789:
I think I'm going to close this because I see that the migration to the new backend is underway, and it feels like I'm not going to be able to get x86_32 support to a usable state before it gets removed.
Wasmtime GitHub notifications bot (Jul 05 2020 at 09:13):tschneidereit commented on Issue #1789:
@whitequark this might make sense, but just to make sure we didn't accidentally create wrong impressions: the old backend framework won't be removed until sometime next year. We'll have switched the default for the x64 codegen to the new framework well before, after which very little will happen around the old one, but it'll stay in-tree for quite a while longer.
Wasmtime GitHub notifications bot (Jul 05 2020 at 09:18):whitequark commented on Issue #1789:
the old backend framework won't be removed until sometime next year
I understand that, but I expect that, once the old backend is no longer heavily tested by the majority of the users, it's going to slowly bit-rot. Since the x86_32 code paths are marginal in first place, it would mean that I'd have to run twice as fast just to remain in the same place, and I don't expect to be able to do that.
Wasmtime GitHub notifications bot (Jul 05 2020 at 09:23):whitequark commented on Issue #1789:
Oh and to add to this: I would be generally happy to work on x32 support in the new backend, but given that it's currently written to support x64 alone, it requires an amount of work I can't justify (and experience with x86 that I do not have). If someone else generalizes the x64 backend to support 32 bits too, like the old one, I'd help bring it to completion.
Wasmtime GitHub notifications bot (Jul 05 2020 at 10:09):tschneidereit commented on Issue #1789:
Since the x86_32 code paths are marginal in first place, it would mean that I'd have to run twice as fast just to remain in the same place, and I don't expect to be able to do that.
I thought that might be the reason, and makes sense to me, yes.
If someone else generalizes the x64 backend to support 32 bits too, like the old one, I'd help bring it to completion.
Much appreciated! I filed #1980 to look into what support for x86 in the new framework could look like.
Wasmtime GitHub notifications bot (Jul 05 2020 at 10:12):whitequark commented on Issue #1789:
I filed #1980 to look into what support for x86 in the new framework could look like.
Thank you! Looking forward to it.
Last updated: Nov 22 2024 at 17:03 UTC