Track variable locations#820
Closed
djolertrk wants to merge 25 commits into0xMiden:nextfrom
Closed
Conversation
Collaborator
Author
|
Goes on top of #812. |
Collaborator
Author
djolertrk
force-pushed
the
feature/debug-variable-locations-clean
branch
3 times, most recently
from
69877ae to
a4ccd95
Compare
Collaborator
Author
|
It depends on the types and debug decorator defined in 0xMiden/miden-vm#2471. |
Collaborator
|
I've done an initial review of the |
djolertrk
force-pushed
the
feature/debug-variable-locations-clean
branch
from
a4ccd95 to
885283c
Compare
This was referenced Jan 19, 2026
djolertrk
force-pushed
the
feature/debug-variable-locations-clean
branch
from
5397179 to
a3fc070
Compare
Collaborator
Author
|
Now rebased on top of #995. |
Collaborator
Author
|
Status on this: VM changes merged |
Contributor
djolertrk
force-pushed
the
feature/debug-variable-locations-clean
branch
from
a3fc070 to
a20d993
Compare
Collaborator
Author
|
rebased on top of #995 |
djolertrk
force-pushed
the
feature/debug-variable-locations-clean
branch
2 times, most recently
from
7647605 to
92b8973
Compare
Collaborator
Author
|
note that it still depends on the local |
djolertrk
force-pushed
the
feature/debug-variable-locations-clean
branch
from
0987c07 to
468609e
Compare
Collaborator
Author
|
NOTE: It uses https://github.com/walnuthq/miden-vm/tree/fix/debug-var-dedup-crash -- but 0xMiden/miden-vm#2955 should be merged soon. |
- Check if WasmLocal value is on Miden operand stack before assuming it's in local memory (emit Stack(pos) if on stack) - Add post-processing in MasmFunctionBuilder::build() to patch Local(idx) with correct FMP offset - Compute num_wasm_locals = params_in_felts + hir_locals for correct offset calculation (DWARF WasmLocal uses WASM indexing where params come first) - Add patch_debug_var_locals_in_block() to recursively patch all DebugVar decorators in the MASM body
Confirm that cfg --> scf preserves debug info
Debug info operations (debuginfo.value, debuginfo.kill, debuginfo.declare) are purely observational and were causing panics and assembler validation errors when they survived through optimization passes into codegen. - Skip operand scheduling for DebugValue (observational, not consuming) - Gracefully skip debuginfo ops with no HirLowering in the emitter - Make SinkOperandDefs and ControlFlowSink debug-info-aware so debug uses don't block sinking or prevent dead code elimination - Strip DebugVar-only procedure bodies that would be rejected by the Miden assembler (decorators without instructions have no MAST effect)
Add end-to-end debug variable location tracking through the compiler pipeline, and patch miden-vm to fix a crash when basic blocks are deduplicated during assembly.
Fix data_offset_align and raw_entity_metadata_layout_for_value_layout to correctly handle types with alignment > 8
- add FrameBase lowering - DWARF parsing (parse variable/params names) - and fix duplicate DebugVar emissions
djolertrk
force-pushed
the
feature/debug-variable-locations-clean
branch
from
73cd2f3 to
4b07c16
Compare
djolertrk
force-pushed
the
feature/debug-variable-locations-clean
branch
from
55b263e to
535185d
Compare
bitwalker
reviewed
Apr 29, 2026
Collaborator
bitwalker
left a comment
bitwalker
left a comment
There was a problem hiding this comment.
Just a heads up @djolertrk, but I'm rebasing this PR on another branch of mine, and cleaning up a few things locally. I'll probably close this PR and open a new one with the updated branch - all your commits are preserved of course, but wanted to let you know what's up so you don't burn cycles on this in the meantime.
Collaborator
|
Closing in favor of #1097 |
bitwalker
added a commit
that referenced
this pull request
May 1, 2026
The initial debug info implementation in #820 defined a new debuginfo dialect which provides the ability to emit tracking information for local variables in the source program. There were a couple of issues that I wanted to address with it before merging, so that has been done here in this commit: * Move the new debug dialect into `midenc-hir`, as debug info attributes are essentially core/builtin attributes, and having them in a downstream dialect was awkward: not only did we need to place them in the `builtin` dialect rather than `di`, but debug info ops are somewhat special in a way that most other ops are not - this warranted making them part of the core IR crate, though still as a separate dialect. * Renamed the dialect to `di` from `debuginfo`, as this better mirrors the naming conventions in DWARF, from which these ops are derived in part. * Renamed the debug dialect attribute types to remove the `DI` prefix, as it is redundant. * Added a new `Transparent` operation trait, which is used to indicate that an operation is "transparent" with regards to its value uses, allowing those uses to be excluded in specific situations, such as when determining liveness of operation results. Transparent ops are only permitted to have at most a single operand, and may not produce results - these restrictions make dealing with transparent ops much more straightforward when rewriting the IR. * Added a new effect type, `DebugEffect`, which is used to represent effects that an operation may have on the state of an attached debugger. This can be used in the future to ensure that operations are not reordered with respect to such effects on a common resource. This largely mirrors MemoryEffect for now. There is a corresponding DebugEffectOpInterface alias added as well. * Made all ops of the debug info dialect implement `Transparent`, as well as both DebugEffectOpInterface and MemoryEffectOpInterface. The memory effects are modeled as empty (i.e. the ops are considered pure), which means that those ops are trivially dead by default - however I have also modified region DCE to explicitly leave transparent ops alone unless we have determined that the defining op of their input operand is dead. * Added a new method to `Value`, called `has_real_uses`, which returns true if the value is used and at least one of those uses is from a non-transparent op. The existing `is_used` method returns true regardless of transparency if any use of the value exists. * Modified various places where `Value::is_used` was being used, when we should use `Value::has_real_uses` to avoid pessimizing a canonicalization based on the presence of debug info ops. * Modified `Rewriter::erase_op` to be more precise about verifying that the op to erase has no _real_ uses. If transparent users still exist, they are removed before erasing the original target op - this is always safe as transparent ops can only ever use a single value. * Added tests to ensure that transparent ops do not interfere with dead code elimination. * Simplified the places where debug ops were special-cased to instead work in terms of transparency (unless the special-casing truly needed to be specific to a given op). * Implemented AttrPrinter/AttrParser for all debug dialect attributes * Implemented OpPrinter/OpParser for all debug dialect operations * Implemented `Serializable`/`Deserializable` for debug dialect's `ExpressionOp` and `Expression` types, so that complex expressions can be serialized for use by the debugger. * Modified how the frontend was computing storage locations to use `Expression` directly, rather than `VariableStorage`, which could not represent the full set of location expressions, and was largely redundant with `ExpressionOp`. We still only handle a subset of possible expression types, but it will be easier to support more going forward.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Track variable locations at HIR and MASM levels.
Add DebugVar decorator and new debug_info section in the final .masp.
Also, add
miden-debugdumptool to dump .debug_info section, so we can explore the custom section from the package.vm part: 0xMiden/miden-vm#2471