Stream: git-wasmtime

Topic: wasmtime / PR #1530 Pending arm64 fixes for Spidermonkey ...

Wasmtime GitHub notifications bot (Apr 17 2020 at 13:06):

bnjbvr opened PR #1530 from bbouvier-arm64-fixes to master:

This rebases and adapts patches that landed in our arm64 work-in-progress branch after the last commit that was merged with the new arm64 backend.

Commit 8f181f8 is a bit invasive, because it propagates the settings::Flags down to the arm64 machinery, and the ABI has to be marked with the Flags' lifetime. I hesitated with just doing a copy, but it's not too bad as is.

Wasmtime GitHub notifications bot (Apr 17 2020 at 13:06):

bnjbvr requested cfallin for a review on PR #1530.

Wasmtime GitHub notifications bot (Apr 17 2020 at 14:51):

bjorn3 submitted PR Review.

Wasmtime GitHub notifications bot (Apr 17 2020 at 14:51):

bjorn3 created PR Review Comment:

    // X22 - X28

Wasmtime GitHub notifications bot (Apr 17 2020 at 18:36):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Apr 17 2020 at 18:36):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Apr 17 2020 at 18:36):

cfallin created PR Review Comment:

Doc comment now that this is pub?

Wasmtime GitHub notifications bot (Apr 17 2020 at 18:36):

cfallin created PR Review Comment:

I understand the optimization here but this Writable cast makes me a little nervous; perhaps it would be better to take a Writable<Reg> for from_reg to enable this? Then the safety argument is a completely local property of the machine-independent lowering code that calls us: it knows that it's generating the copy into the retval just before the epilogue, and so can break its usual "SSA value's register is only writable at SSA def point" invariant (or if it does something different, allocate a writable temp to use here).

Wasmtime GitHub notifications bot (Apr 17 2020 at 18:36):

cfallin created PR Review Comment:

Can we just create an Inst::Move and then inst.emit(sink); as we do for the other pseudoinstructions?

Wasmtime GitHub notifications bot (Apr 17 2020 at 18:36):

cfallin created PR Review Comment:

I'm not super-excited about the 'flags lifetime carried into the VCode; it will propagate to anything else that contains a VCode which is not very ergonomic. (On preview of the rest of the patch below, it already led to a lot of boilerplate churn.) Reading the generated code, it looks like it's currently a [u8; 7] (so the pointer is larger than the flags array itself!). In general, with a bit per flag, I wouldn't expect it to grow beyond a few words, for a while anyway. Could we just clone it instead?

Wasmtime GitHub notifications bot (Apr 21 2020 at 10:01):

bnjbvr submitted PR Review.

Wasmtime GitHub notifications bot (Apr 21 2020 at 10:01):

bnjbvr created PR Review Comment:

Oh yes, it's much simpler.

Wasmtime GitHub notifications bot (Apr 21 2020 at 10:01):

bnjbvr updated PR #1530 from bbouvier-arm64-fixes to master:

This rebases and adapts patches that landed in our arm64 work-in-progress branch after the last commit that was merged with the new arm64 backend.

Commit 8f181f8 is a bit invasive, because it propagates the settings::Flags down to the arm64 machinery, and the ABI has to be marked with the Flags' lifetime. I hesitated with just doing a copy, but it's not too bad as is.

Wasmtime GitHub notifications bot (Apr 21 2020 at 10:02):

bnjbvr requested cfallin for a review on PR #1530.

Wasmtime GitHub notifications bot (Apr 21 2020 at 10:13):

bnjbvr updated PR #1530 from bbouvier-arm64-fixes to master:

This rebases and adapts patches that landed in our arm64 work-in-progress branch after the last commit that was merged with the new arm64 backend.

Commit 8f181f8 is a bit invasive, because it propagates the settings::Flags down to the arm64 machinery, and the ABI has to be marked with the Flags' lifetime. I hesitated with just doing a copy, but it's not too bad as is.

Wasmtime GitHub notifications bot (Apr 21 2020 at 10:20):

bjorn3 created PR Review Comment:

Should this be updated?

Wasmtime GitHub notifications bot (Apr 21 2020 at 10:20):

bjorn3 submitted PR Review.

Wasmtime GitHub notifications bot (Apr 21 2020 at 10:21):

bjorn3 edited PR Review Comment.

Wasmtime GitHub notifications bot (Apr 21 2020 at 15:07):

cfallin submitted PR Review.

Wasmtime GitHub notifications bot (Apr 21 2020 at 15:08):

cfallin merged PR #1530.

Last updated: Dec 23 2024 at 13:07 UTC