[DebugInfo][X86] Improve X86 Optimize LEAs handling of debug values.
Needs ReviewPublic

Authored by andrewng on Mon, Apr 3, 8:35 AM.

Details

Summary

This is a follow up to the fix in r298360 to improve the handling of debug values when redundant LEAs are removed. The fix in r298360 effectively discarded the debug values. This patch now attempts to preserve the debug values by using the Dwarf 4 DW_OP_stack_value operation.

Diff Detail

andrewng created this revision.Mon, Apr 3, 8:35 AM
aprantl added inline comments.Mon, Apr 3, 9:08 AM
lib/Target/X86/X86OptimizeLEAs.cpp
267

No need to spell out \brief. LLVM uses autobrief in the doxygen settings.

545

this comment should go into the header instead.

556

This is for the backend to decide and discard. We shouldn't have any DWARF version specific code here. (E.g.: what if we generate CodeView debug info?)

After landing https://reviews.llvm.org/D31439, DwarfExpression can actually drop locations that can not be expressed in the requested DWARF version.

andrewng added inline comments.Mon, Apr 3, 10:57 AM
lib/Target/X86/X86OptimizeLEAs.cpp
545

Do you mean the class definition?

556

OK, so I should just generate the DIExpression with the DW_OP_stack_value operation and the backend will discard the DBG_VALUE if the DWARF version is not compatible (in the case of DWARF debug info)?

aprantl added inline comments.Mon, Apr 3, 11:11 AM
lib/Target/X86/X86OptimizeLEAs.cpp
556

Yes, please. Note that the backend cannot (yet) doe this, but I will hopefully be able to land this functionality this week.

andrewng updated this revision to Diff 94056.Tue, Apr 4, 5:38 AM

Removed DWARF version checks as suggested by Adrian (thanks) and updated comments.

aprantl added inline comments.Thu, Apr 20, 9:41 AM
lib/Target/X86/X86OptimizeLEAs.cpp
550

Is AddrDispShift always > 0?

aprantl added inline comments.Thu, Apr 20, 2:02 PM
lib/Target/X86/X86OptimizeLEAs.cpp
556

This change is finally in r300882.

andrewng added inline comments.Fri, Apr 21, 3:02 AM
lib/Target/X86/X86OptimizeLEAs.cpp
550

I believe that AddrDispShift can be either positive or negative. I assume that it won't be zero as that would imply there were two "identical" LEAs.

aprantl added inline comments.Fri, Apr 21, 8:53 AM
lib/Target/X86/X86OptimizeLEAs.cpp
550

Okay then I think there should be an assert(AddrDispShift != 0 && "...") here to make sure we change the ?: condition should this ever change in the future.

559

Are you sure you want the DW_OP_stack_value at position 0? Shouldn't it go either at the end of the expression or just before a DW_OP_llvm_fragment operation?

561

I think I would prefer std::next() over the +1.