Stream: git-wasmtime

Topic: wasmtime / PR #9576 Remove use of `MemoryStyle` when comp...

Wasmtime GitHub notifications bot (Nov 06 2024 at 19:54):

alexcrichton requested fitzgen for a review on PR #9576.

Wasmtime GitHub notifications bot (Nov 06 2024 at 19:54):

alexcrichton requested wasmtime-core-reviewers for a review on PR #9576.

Wasmtime GitHub notifications bot (Nov 06 2024 at 19:54):

alexcrichton opened PR #9576 from alexcrichton:cranelift-no-memorystyle to bytecodealliance:main:

Instead read directly from tunables and MemoryType with new helper methods that can be shared between Cranelift, Winch, and the rest of the memory subsystem.

Note that this is intended to be a pure-refactoring change. The diff here is large-ish but it's mostly accounted for via code movement and indentation changes. The high-level changes made to the structure of the code are:

• Metadata for PCC is de-indented and uses similar coarse determination for what facts to attach as before. Note that there's still a disconnect between PCC facts being applied and the actual load/store itself and so fully supporting PCC will probably require more refactoring in the future.

• Cases have been reordered for actually emitting a bounds check. Due to there no longer being a top-level static/dynamic branch the cases have been reordered in terms of priority -- for example unconditional trapping is first, then elision of bounds checks, then the assumption the bound limit is a constant, etc.

• Cases for bounds checks have had their arms rewritten in terms of the new properties. The main new one is that the fallback case for static memories previously which had a bounds check is a little more complicated as it additionally factors in memory_may_move in addition to memory_reservation. This is captured in the expanded documentation for this case, however.

• Documentation was updated to avoid talking about static/dynamic memories.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Nov 06 2024 at 23:04):

fitzgen submitted PR review.

Wasmtime GitHub notifications bot (Nov 06 2024 at 23:04):

fitzgen created PR review comment:

If this sequence is not going to be a match anymore, could we do a sequence of ifs that return and then a final base case? I think in this particular case, where we have big comments explaining each case, it will be more clear:

// Big comment...
if cond() && foo <= bar {
    // ...
    return ...;
}

// Special case for when blah blah ....
if ... {
    // ...
    return ...
}

// Base case: ...
...
Reachable

Wasmtime GitHub notifications bot (Nov 07 2024 at 01:13):

alexcrichton updated PR #9576.

Wasmtime GitHub notifications bot (Nov 07 2024 at 01:13):

alexcrichton has enabled auto merge for PR #9576.

Wasmtime GitHub notifications bot (Nov 07 2024 at 01:39):

alexcrichton merged PR #9576.

Last updated: Dec 23 2024 at 12:05 UTC