This is an archive of the discontinued LLVM Phabricator instance.

[Local] Do not ignore zexts in salvageDebugInfo, PR45923
ClosedPublic

Authored by vsk on May 15 2020, 2:03 PM.

Details

Summary

When salvaging a dead zext instruction, append a convert operation to
the DIExpressions of the debug uses of the instruction, to prevent the
salvaged value from being sign-extended.

I confirmed that lldb prints out the correct unsigned result for "f" in
the example from PR45923 with this changed applied.

Diff Detail

Event Timeline

vsk created this revision.May 15 2020, 2:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 15 2020, 2:03 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
aprantl accepted this revision.May 15 2020, 2:11 PM

Great!

llvm/lib/Transforms/Utils/Local.cpp
1702

I'm pretty sure this condition predates DW_OP_convert.

This revision is now accepted and ready to land.May 15 2020, 2:11 PM
aprantl added inline comments.May 15 2020, 2:14 PM
llvm/test/Transforms/InstCombine/cast-mul-select.ll
178

I have not thought about the exact semantics, but I'm wondering if it would be possible to contract a sequence on DW_OP_convert operations in the backend in DwarfExpression.cpp to have a smaller encoding? I guess this would be easier if we distinguished between extending and truncating a value in our IR. Perhaps we'd need to introduce DW_OP_LLVM_zext & friends?

vsk marked an inline comment as done.May 15 2020, 5:38 PM
vsk added subscribers: JDevlieghere, debug-info.
vsk added inline comments.
llvm/test/Transforms/InstCombine/cast-mul-select.ll
178

Yeah, we could try to simplify dwarf expressions really late in the backend, and/or do something like DW_OP_LLVM_zext to avoid emitting silly expressions in the first place. The former might allow us to compact-ify dwarf in more cases where there's emergent complexity in the expression, but is also quite possibly overkill (cc @JDevlieghere). We should probably look at what kind of expressions actually pop out of a stage2 build, then figure out where (if anywhere) we ought to make things more compact.

This revision was automatically updated to reflect the committed changes.