[PATCH] Debug info: Support fragmented variables.

[Adrian Prantl]

  On Feb 26, 2014, at 3:06, Chandler Carruth <chandlerc at gmail.com> wrote:

  Hi Chandler/Eric/David,

  Thanks for your feedback, everyone!
  I think I addressed every comment now, below are some notes regarding the non-obvious or controversial changes.

  > Two high-level comments:

  > 1) This needs a set of direct SROA tests.
  There are now three tests that explicitly run opt -sroa and check the IR output. The IR output is then also compiled with llc to verify the dwarf output, so they are still located in the tests/DebugInfo directory.

  > Detailed questions below. The most important one is trying to understand how you handle un-split cases and iteratively split cases.
  > ...
  > What happens if the original store was already a split? This will happen a lot with SROA when we iterate on the same alloca.

  This is not an issue because we can create a variable piece from an existing variable piece just fine.

  > Also, what about when this *isn't* a split at all? I feel like the first patch should just migrate dbg.declares from old allocas to new allocas when we're rewriting uses but not re-slicing anything.

  I moved the code that migrates the debug declares from the Store handler to the AllocaSlice visitor of AllocaRewriter. It also handles the case were the alloca is just rewritten without slicing. Currently it’s using the DIType of the DIVariable to determine whether this should be a VariablePiece or not. This is not strictly necessary, though: it forces us to build a DITypeIdentifierMap in SROA, but it saves us 3 int32 per variable MDNode when the alloca is not split. Alternatively we could also just always create pieces, and perform the check whether a piece covers the entire variable back in AsmPrinter.

  ================
  Comment at: lib/Transforms/Scalar/SROA.cpp:426-429
  @@ -419,1 +425,6 @@

  +    // Make a best effort to find a dbg.declare intrinsic describing
  +    // the alloca by peeking at the next instruction.
  +    if (DbgDeclareInst *DDI=dyn_cast_or_null<DbgDeclareInst>(SI.getNextNode()))
  +      S.DbgDeclare = DDI;
  +
  ————————
  > This seems like a total hack.

  > It papers over the fact that this is not relevant for a particular store, but for the entire alloca.

  > Why can't we find this by locating the use of the alloca in the metadata and then the intrinsic using the metadata? > I know it's a non-standard edge in the IR, but I was pretty sure it was still *an* edge in the IR somewhere that would let us find this?

  The current implementation uses a DenseMap<AllocaInst, DbgDeclare> to find the debug intrinsic for an alloca.


  ================
  Comment at: lib/Transforms/Scalar/SROA.cpp:2241-2244
  @@ +2240,6 @@
  +      if (Var.getType().getSizeInBits() < Size*8 /* Assuming 8 Bits/Byte */) {
  +        DIVariable Piece = DIB.createVariablePiece(Var, BeginOffset, Size);
  +        DIB.insertDbgValueIntrinsic(V, 0, Piece, Store)
  +          ->setDebugLoc(DbgDecl->getDebugLoc());
  +        Pass.DeadInsts.insert(DbgDecl);
  +      }
  ————————
  > I'm probably missing something, but it *looks* like this has the real possibility of taking code which has no partial info (no dbg.value calls) and making it suddenly have more info. Is that right? Is that good?

  Maybe I’m misunderstanding what you said there, but as far as I understand it this:
  > Should we be looking for relevant dbg.value calls pertaining to the original alloca and slicing them up rather than just synthesizing our own?
  is describing exactly what we are doing right now. Also note that only dbg.declare instrinsics can describe allocas.

  > Also, what about memset? memcpy? speculated stores around phis? I assume follow-up patches, but it might be good to document some of that.

  This is not yet complete and I plan to add other cases over time!

Hi #llvm,

http://llvm-reviews.chandlerc.com/D2680

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D2680?vs=7324&id=7571#toc

Files:
  docs/SourceLevelDebugging.rst
  include/llvm/CodeGen/AsmPrinter.h
  include/llvm/CodeGen/MachineInstr.h
  include/llvm/CodeGen/SelectionDAG.h
  include/llvm/DIBuilder.h
  include/llvm/DebugInfo.h
  lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  lib/CodeGen/AsmPrinter/DwarfDebug.h
  lib/CodeGen/AsmPrinter/DwarfUnit.cpp
  lib/CodeGen/SelectionDAG/InstrEmitter.cpp
  lib/CodeGen/SelectionDAG/LegalizeTypes.cpp
  lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  lib/IR/DIBuilder.cpp
  lib/IR/DebugInfo.cpp
  lib/Transforms/Scalar/SROA.cpp
  test/DebugInfo/X86/dbg-large-unsigned-const.ll
  test/DebugInfo/X86/pieces-1.ll
  test/DebugInfo/X86/sdagsplit-1.ll
  test/DebugInfo/X86/sdagsplit-2.ll
  test/DebugInfo/X86/sroasplit-1.ll
  test/DebugInfo/X86/sroasplit-2.ll
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2680.5.patch
Type: text/x-patch
Size: 69233 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140305/23a7c5ae/attachment.bin>