Wasmtime GitHub notifications bot (Jan 16 2024 at 23:13): Wasmtime GitHub notifications bot (Jan 16 2024 at 23:13):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
retThe reason behind
2is that often instructions following aretwill 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
-->
Wasmtime GitHub notifications bot (Jan 16 2024 at 23:13):elliottt requested fitzgen for a review on PR #7782.
Wasmtime GitHub notifications bot (Jan 16 2024 at 23:13):elliottt requested wasmtime-compiler-reviewers for a review on PR #7782.
Wasmtime GitHub notifications bot (Jan 16 2024 at 23:14):elliottt requested saulecabrera for a review on PR #7782.
Wasmtime GitHub notifications bot (Jan 16 2024 at 23:29):elliottt updated PR #7782.
Wasmtime GitHub notifications bot (Jan 17 2024 at 00:06):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
retThe reason behind
2is that often instructions following aretwill 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
-->
Wasmtime GitHub notifications bot (Jan 17 2024 at 00:07):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
retThe reason behind
2is that often instructions following aretwill 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
-->
Wasmtime GitHub notifications bot (Jan 17 2024 at 02:45):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>
Wasmtime GitHub notifications bot (Jan 17 2024 at 12:59):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.
Wasmtime GitHub notifications bot (Jan 17 2024 at 13:06):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.
Wasmtime GitHub notifications bot (Jan 17 2024 at 13:06):saulecabrera submitted PR review.
Wasmtime GitHub notifications bot (Jan 17 2024 at 16:41):elliottt submitted PR review.
Wasmtime GitHub notifications bot (Jan 17 2024 at 16:41):elliottt created PR review comment:
That's a great idea, I'll plumb through a flag :+1:
Wasmtime GitHub notifications bot (Jan 17 2024 at 16:56):elliottt updated PR #7782.
Wasmtime GitHub notifications bot (Jan 17 2024 at 17:57):elliottt merged PR #7782.
Last updated: Nov 22 2024 at 16:03 UTC