Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DebugLocationPropagation: pass debuglocation from parent node to chil… #6500

Merged
merged 5 commits into from
Apr 21, 2024

Conversation

XMadrid
Copy link
Contributor

@XMadrid XMadrid commented Apr 15, 2024

This PR creates a pass to propagate debug location from parent node to child nodes which has no debug location with pre-order traversal. This is useful for compilers that use Binaryen API to generate WebAssembly modules.

It behaves like wasm-opt read text format file: children are tagged with the debug info of the parent, if they have no annotation of their own.

For compilers that use Binaryen API to generate WebAssembly modules, it is a bit redundant to add debugInfo for each expression, Especially when the compiler wrap expressions.

With this pass, compilers just need to add debugInfo for the parent node, which is more convenient.

For example:

(drop
  (call $voidFunc)
)

Without this pass, if the compiler only adds debugInfo for the wrapped expression drop, the call expression has no corresponding source code mapping in DevTools debugging, which is obviously not user-friendly.

@kripken
Copy link
Member

kripken commented Apr 15, 2024

This makes sense to me to add, I agree it looks useful for compilers.

Before I look at the code in detail, though, @tlively didn't you have plans to add something like this, to copy debug info from parents to (unmarked) children? Maybe for the new parser somehow? If so perhaps code could be shared here.

@tlively
Copy link
Member

tlively commented Apr 16, 2024

@kripken
Copy link
Member

kripken commented Apr 16, 2024

Thanks @tlively !

Then @XMadrid I think the best thing would be to refactor that existing code out into a pass, and use the pass in the current place.

@XMadrid
Copy link
Contributor Author

XMadrid commented Apr 16, 2024

OK, Thank you! I will move existing code out to a pass. And then we can use this pass in wasm-opt options and wat-parser.

@XMadrid
Copy link
Contributor Author

XMadrid commented Apr 17, 2024

Hi, I refactored the existing code into a pass. Use a stack to keep track of traversed expressions, which has a bit lower space complexity. The functionality should be the same. Could you please kindly review this @kripken @tlively ?

src/passes/DebugLocationPropagation.cpp Show resolved Hide resolved
src/passes/pass.cpp Outdated Show resolved Hide resolved
Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

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

Thanks, I have a better undstanding of how this works now.

src/passes/DebugLocationPropagation.cpp Outdated Show resolved Hide resolved
src/passes/DebugLocationPropagation.cpp Outdated Show resolved Hide resolved
test/lit/passes/debug-location-propagation.wast Outdated Show resolved Hide resolved
@tlively tlively enabled auto-merge (squash) April 19, 2024 20:09
@XMadrid
Copy link
Contributor Author

XMadrid commented Apr 20, 2024

Thanks, I think this PR can be merged.

@tlively tlively merged commit 7b7832d into WebAssembly:main Apr 21, 2024
13 checks passed
@tlively
Copy link
Member

tlively commented Apr 21, 2024

Oops, I had forgotten to hit the approve button. Thanks for working on this!

@gkdn gkdn mentioned this pull request Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants