Skip to content

winch: Reduce instruction offset printing in winch#7782

Merged
elliottt merged 3 commits into
bytecodealliance:mainfrom
elliottt:trevor/winch-no-asm-offsets
Jan 17, 2024
Merged

winch: Reduce instruction offset printing in winch#7782
elliottt merged 3 commits into
bytecodealliance:mainfrom
elliottt:trevor/winch-no-asm-offsets

Conversation

@elliottt
Copy link
Copy Markdown
Member

@elliottt elliottt commented Jan 16, 2024

In order to make updates to winch filetess a bit less noisy, restrict isntruction offset printing to two cases:

  1. The beginning of a basic block (omitting the first block)
  2. All instructions after a ret

The reason behind 2 is that often instructions following a ret 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

@elliottt elliottt requested a review from a team as a code owner January 16, 2024 23:13
@elliottt elliottt requested review from fitzgen and saulecabrera and removed request for a team January 16, 2024 23:13
Only emit instruction offsets at basic block boundaries, or for all
instructions after a return. The reason behind the latter, is that many
of the instructions after a return are traps, and will be common jump
targets.
@elliottt elliottt force-pushed the trevor/winch-no-asm-offsets branch from fae7bda to e030db8 Compare January 16, 2024 23:29
@github-actions github-actions Bot added the winch Winch issues or pull requests label Jan 17, 2024
@github-actions
Copy link
Copy Markdown

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 .github/subscribe-to-label.json configuration file.

Learn more.

Copy link
Copy Markdown
Member

@saulecabrera saulecabrera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread winch/filetests/src/disasm.rs
@elliottt elliottt added this pull request to the merge queue Jan 17, 2024
Merged via the queue into bytecodealliance:main with commit 4b2425d Jan 17, 2024
@elliottt elliottt deleted the trevor/winch-no-asm-offsets branch January 17, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

winch Winch issues or pull requests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve Winch's file tests disassembly

2 participants