Stream: git-wasmtime

Topic: wasmtime / PR #1900 MachInst isel and aarch64 backend: do...


view this post on Zulip Wasmtime GitHub notifications bot (Jun 18 2020 at 18:33):

cfallin opened PR #1900 from clarify-lowering-docs to master:

From discussion with Julian and Ben, this PR makes a few documentation-
and naming-level changes (no functionality change):

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Jun 18 2020 at 18:33):

cfallin requested bnjbvr and julian-seward1 for a review on PR #1900.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 18 2020 at 18:33):

cfallin requested bnjbvr and julian-seward1 for a review on PR #1900.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 18 2020 at 19:21):

cfallin updated PR #1900 from clarify-lowering-docs to master:

From discussion with Julian and Ben, this PR makes a few documentation-
and naming-level changes (no functionality change):

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

Please ensure all communication adheres to the code of conduct.
-->

view this post on Zulip Wasmtime GitHub notifications bot (Jun 19 2020 at 05:05):

julian-seward1 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 19 2020 at 05:05):

julian-seward1 created PR Review Comment:

Indeed, IIUC, it is guaranteed not to overlap with any of the inputs of either this or any other IR instruction, yes?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 19 2020 at 05:05):

julian-seward1 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2020 at 12:44):

bnjbvr submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2020 at 12:44):

bnjbvr submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2020 at 12:44):

bnjbvr created PR Review Comment:

This output IR inst could be the input to another IR inst, right? And we're only concerned about usage local to the current IR instruction here. Maybe I'm missing something here?

view this post on Zulip Wasmtime GitHub notifications bot (Jun 22 2020 at 12:44):

bnjbvr created PR Review Comment:

Thanks for this renaming which clarifies the side-effects happening here indeed.

As an alternative, as demonstrated by the changes in this PR, it seems that the narrowing is the edge case, and not the usual thing to do.

Two propositions:

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2020 at 16:54):

cfallin submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2020 at 16:54):

cfallin created PR Review Comment:

Yes, as @bnjbvr said, the guarantee is only in the context of the current instruction being lowered (since if it were never used by any other IR instruction either, the computation would be dead).

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2020 at 16:57):

cfallin submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2020 at 16:57):

cfallin created PR Review Comment:

Sure, I would be fine with the latter; perhaps we can consider this in the context of refactoring to share helpers with the x64 backend? (I'm thinking that e.g. we could define a trait to describe codegen for value widening and then plug that into a generic helper, for example. Similar for codegenning constants. Part of a "libMachBackend" of sorts as an optional layer on top of the lowering context API.)

view this post on Zulip Wasmtime GitHub notifications bot (Jun 23 2020 at 16:59):

cfallin merged PR #1900.

view this post on Zulip Wasmtime GitHub notifications bot (Jun 24 2020 at 09:09):

bnjbvr created PR Review Comment:

Good idea!

view this post on Zulip Wasmtime GitHub notifications bot (Jun 24 2020 at 09:09):

bnjbvr submitted PR Review.


Last updated: Dec 23 2024 at 13:07 UTC