Stream: git-wasmtime

Topic: wasmtime / PR #7304 A couple of small refactorings to the...

Wasmtime GitHub notifications bot (Oct 19 2023 at 22:52):

elliottt opened PR #7304 from elliottt:trevor/elaborate-fixes to bytecodealliance:main:

@jameysharp and I noticed a couple of refactoring opportunities while reading through the elaboration pass:

• The elaboration loop doesn't need to match on the top of the stack as a reference, because each case pops it immediately. Instead we can pop and match on the popped value.
• Computing the cost of a Result value that's not in the DFG was using the block that contains the instruction to determine the level, but since the instruction is already known to not be in the DFG, this would default to LoopLevel::root() unconditionally. This also meant that the Cost::at_level function turned into an identity function on the cost given, making it unnecessary.

Co-authored-by: Jamey Sharp <jsharp@fastly.com>

<!--
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 (Oct 19 2023 at 22:52):

elliottt requested abrown for a review on PR #7304.

Wasmtime GitHub notifications bot (Oct 19 2023 at 22:52):

elliottt requested wasmtime-compiler-reviewers for a review on PR #7304.

Wasmtime GitHub notifications bot (Oct 19 2023 at 22:53):

elliottt requested cfallin for a review on PR #7304.

Wasmtime GitHub notifications bot (Oct 19 2023 at 23:16):

elliottt edited PR #7304:

@jameysharp and I noticed a couple of refactoring opportunities while reading through the elaboration pass:

• The elaboration loop doesn't need to match on the top of the stack as a reference, because each case pops it immediately. Instead we can pop and match on the popped value.
• Computing the cost of a pure ValueDef::Result was using the block that contains the instruction to determine the level, but since pure values will not be in the DFG yet, this would default to LoopLevel::root() unconditionally. This also meant that the Cost::at_level function turned into an identity function on the cost given, making it unnecessary.

Co-authored-by: Jamey Sharp <jsharp@fastly.com>

<!--
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 (Oct 19 2023 at 23:52):

cfallin submitted PR review:

LGTM! Good find w.r.t. the loop-level cost factor being a no-op; I suspect this was left over from an earlier version of extraction. I'd be interested to hear any thoughts you or others have about using the loop level somehow, but I'm happy to see this merged for now!

Wasmtime GitHub notifications bot (Oct 20 2023 at 00:36):

cfallin merged PR #7304.

Last updated: Dec 23 2024 at 13:07 UTC