-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Fixing source map locations when peephole optimizations are applied. #15384
Conversation
⏱️ 2h 55m total CI duration on this PR
🚨 1 job on the last run was significantly faster/slower than expected
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
a21b337
to
e4f7584
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks ok, though I'd at least add a comment for code clarity.
third_party/move/move-ir-compiler/move-bytecode-source-map/src/source_map.rs
Outdated
Show resolved
Hide resolved
...arty/move/move-compiler-v2/src/file_format_generator/peephole_optimizer/inefficient_loads.rs
Show resolved
Hide resolved
e4f7584
to
33c6f61
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
✅ Forge suite
|
Description
Fixes #14167.
Peephole optimization changes the original file format bytecode after it is generated from stackless bytecode. Source mapping is computed when the original file format bytecode is created, so the source mapping is incorrect after peephole optimization changes the file format bytecode.
This resulted in several downstream issues, including move test, prover, coverage, etc.
This PR contains the fix by having the peephole optimization maintain a mapping to the original offsets and propagating them, and updating the source map accordingly after the optimization.
How Has This Been Tested?
When we ran
aptos move test --move-2
for the following program:on the
main
branch, we get:with this PR, we get (correctly):
Key Areas to Review
Type of Change
Which Components or Systems Does This Change Impact?