Replace hand-rolled DI scope rewriter with mlir::AttrTypeReplacer#199
Merged
Conversation
LLVMDebugInfoHelperFixer completes each function's DISubprogramAttr with real signature DI types at lowering time (they need LLVM layout info that does not exist at MLIRGen time), then must remap every reference to the old scope. That remapping was ~450 lines of hand-written recursive location/attribute rewriting; mlir::AttrTypeReplacer does the same job generically with a single DISubprogramAttr replacement rule, rebuilding any attribute or location that contains the old scope transitively. Also fixes real defects the old rewriter had: - CallSiteLoc handling compared the new caller against getCallee() and never rewrote callee scopes, leaking stale empty-signature subprograms into DWARF as duplicate DISubprogram nodes (3-7 duplicates per test file in 00class/00funcs/02numbers --di output; gone after this change, otherwise byte-identical debug info modulo nondeterministic name hashes). - The DbgValueOp branch compared against dbgDeclare (null there), a latent null-op dereference. - Unknown Location kinds hit llvm_unreachable; AttrTypeReplacer handles all location kinds. Also removes the never-referenced LLVMDebugInfoHelperCreator and the unused stripMetadata/removeAllMetadata/walkMetadata/debugPrint helpers. Verified: --di LLVM IR compared before/after on 00funcs, 00class, 19forof_capture, 02numbers (identical except duplicate removal); full test suite 683/683; JIT smoke run with --di. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
LLVMDebugInfoHelperFixercompletes each function'sDISubprogramAttrwith real signature DI types at lowering time (they need LLVM layout info that doesn't exist at MLIRGen time), then must remap every reference to the old scope. That remapping was ~450 lines of hand-written recursive location/attribute rewriting;mlir::AttrTypeReplacerdoes the same job generically with a singleDISubprogramAttrreplacement rule, transitively rebuilding any attribute or location containing the old scope (lexical blocks, local variables, labels, nested subprograms).Net: 61 insertions, 456 deletions in
LLVMDebugInfoFixer.h.Bugs the old rewriter had (fixed by this change)
CallSiteLoccase compared the new caller againstgetCallee()and never rewrote callee scopes, leaking stale empty-signature subprograms into DWARF as duplicateDISubprogramnodes (3–7 duplicates per file observed in--dioutput of 00class/00funcs/02numbers; gone after this change).DbgValueOpbranch compared againstdbgDeclare(null in that branch) — latent null-op dereference.Locationkinds hitllvm_unreachable;AttrTypeReplacerhandles all location kinds generically.Also removes the never-referenced
LLVMDebugInfoHelperCreatorand unusedstripMetadata/removeAllMetadata/walkMetadata/debugPrinthelpers.Test plan
--diLLVM IR compared before/after on 00funcs, 00class, 19forof_capture, 02numbers: identical except removal of the duplicateDISubprogramnodes (modulo pre-existing nondeterministic name hashes, confirmed by same-binary self-diff).--dienabled.🤖 Generated with Claude Code