[PATCH] D45878: [DEBUG INFO] Fixing cases where debug info (-g) causes changes in the program.

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa created this revision.
jonpa added reviewers: uweigand, echristo.
Herald added subscribers: JDevlieghere, javed.absar, aprantl, MatzeB.

I tried -enable-ipra on SPEC (SystemZ) and got three benchmark crashes, which disappeared with -g. The explanation I got on this is that if debug-info changes the program, it is a bug. Optimizers should ignore debug-info. Indeed, when looking into this, I found many cases where debug info instructions (/intrinsics) affect the optimizers because they forgot to e.g. iterate past a debug-instruction (which uses a register).

I have gotten to a point where my original crash with -enable-ipra can be compiled with -g and debugged (identical text sections). There are still more cases (~10 files) when building SPEC-2006 with -O3 with and w/out -g, that do not have identical output with objdump -d on object files. I have so far handled only cases that came up, except the SystemZ CondTrap which was handled on the fly, when handling the BRCT conversion.

I am not 100% sure what is expected for DBG_VALUEs or (llvm.dbg.value intrinsics), so I have merely begun by mimicking a handling I found in MachineSink.cpp. Since I didn't know where to add this as a utility function, I have now a lot of local copies of that function, which of course should be fixed.

I have assumed:

1. If an instruction is moved, then the immediately following DBG_VALUE(s) that read the defined register is also moved with it.

2. If an instruction is erased, those DBG_VALUE(s) would also be erased.

So far, I have found use for these new utility functions: collectDebugValues, moveDebugValues, eraseDebugValues. I would like to put these in as utilities somewhere, and use them everywhere. The caller should own the DbgValues vector, I think. Is there a file I could put this in, or perhaps it could be part of MachineInstr/MachineBasicBlock?

TODO:

1. utility functions instead of copied code. Reuse also in MachineSink.cpp (both pre/post RA).
2. tests: Can we test explicitly that -g does not alter the text section, like via objdump two times (with and w/out -g) into temporary files, or do I need to make clever tests that checks the specific codegen that I found?
3. This seems wildly neglected (as it is in gcc, I hear). What could we do?


https://reviews.llvm.org/D45878

Files:
  lib/CodeGen/CodeGenPrepare.cpp
  lib/CodeGen/MachineCopyPropagation.cpp
  lib/CodeGen/MachineSink.cpp
  lib/CodeGen/TwoAddressInstructionPass.cpp
  lib/Target/SystemZ/SystemZElimCompare.cpp
  lib/Target/SystemZ/SystemZMachineScheduler.cpp
  lib/Transforms/IPO/FunctionAttrs.cpp
  lib/Transforms/Scalar/MergedLoadStoreMotion.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D45878.143301.patch
Type: text/x-patch
Size: 11556 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180420/e3392f6c/attachment.bin>