elliottt opened PR #7782 from elliottt:trevor/winch-no-asm-offsets
to bytecodealliance:main
:
In order to make updates to winch filetess a bit less noisy, restrict isntruction offset printing to two cases:
- The beginning of a basic block (omitting the first block)
- All instructions after a
ret
The reason behind
2
is that often instructions following aret
will be part of the trap table, and will thus be a common jump target from other areas of the function.<!--
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.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
elliottt requested fitzgen for a review on PR #7782.
elliottt requested wasmtime-compiler-reviewers for a review on PR #7782.
elliottt requested saulecabrera for a review on PR #7782.
elliottt updated PR #7782.
elliottt edited PR #7782:
In order to make updates to winch filetess a bit less noisy, restrict isntruction offset printing to two cases:
- The beginning of a basic block (omitting the first block)
- All instructions after a
ret
The reason behind
2
is that often instructions following aret
will be part of the trap table, and will thus be a common jump target from other areas of the function.Fixes #7552
<!--
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.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
elliottt edited PR #7782:
In order to make updates to winch filetess a bit less noisy, restrict isntruction offset printing to two cases:
- The beginning of a basic block (omitting the first block)
- All instructions after a
ret
The reason behind
2
is that often instructions following aret
will be part of the trap table, and will thus be a common jump target from other areas of the function.This approach isn't perfect, as jumping over single instructions won't end up forcing the destination to have a label. However, it does get most cases right, and makes changes to the disassembly much smaller.
Fixes #7552
<!--
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.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
github-actions[bot] commented on PR #7782:
Subscribe to Label Action
cc @saulecabrera
<details>
This issue or pull request has been labeled: "winch"Thus the following users have been cc'd because of the following labels:
- saulecabrera: winch
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
saulecabrera submitted PR review:
Thanks for working on this. I agree that the diff is now less noisy and that in most cases it's easy to verify by looking what's going on in the disassembly.
saulecabrera created PR review comment:
Do you think it'll be worth conditionally ignoring the offsets only when the disassembly is invoked from the filetests? This disassembly utility is also used by
winch-tools compile <file.wat>
, in which case it could be useful to print the offsets for a more in-depth analysis.
saulecabrera submitted PR review.
elliottt submitted PR review.
elliottt created PR review comment:
That's a great idea, I'll plumb through a flag :+1:
elliottt updated PR #7782.
elliottt merged PR #7782.
Last updated: Dec 23 2024 at 12:05 UTC