[PATCH] Debug info: Support fragmented variables.

[Adrian Prantl]

================
Comment at: lib/Transforms/Scalar/SROA.cpp:3265-3267
@@ +3264,5 @@
+      DIExpression Piece;
+      if (DL->getTypeAllocSize(SliceTy) !=
+          DL->getTypeAllocSize(AI.getAllocatedType()))
+        Piece = DIB->createPieceExpression(BeginOffset, SliceSize);
+      Instruction *NewDDI = DIB->insertDeclare(NewAI, Var, Piece, &AI);
----------------
chandlerc wrote:
> This predicate is somewhat subtle. You're creating a piece expression any time the alloc size isn't the same, but you're using the slice size rather than the alloc size to create the piece.... Can you add a comment here? If I came to this code later, I wouldn't understand why the different sizes used were significant.
I changed condition and comment to this, which should be more clear:


```
      // Create a piece expression describing the slice, if the new slize is
      // smaller than the old alloca or the old alloca already was described
      // with a piece. It would be even better to just compare against the size
      // of the type described in the debug info, but then we would need to
      // build an expensive DIRefMap.
      if (SliceSize < DL->getTypeAllocSize(AI.getAllocatedType()) ||
          DbgDeclares[AI].getExpression().isVariablePiece())
        Piece = DIB->createPieceExpression(BeginOffset, SliceSize);

```

================
Comment at: lib/Transforms/Scalar/SROA.cpp:3728
@@ -3703,1 +3727,3 @@
+
+  DIB.reset(new DIBuilder(*F.getParent(), /*AllowUnresolved*/ false));
 
----------------
chandlerc wrote:
> Is there a particular cost to constructiog DIBuilder objects? I ask because, were this an IRBuilder, I would have said to just construct it directly in each place we need it, because there is no real cast associated with it. If the same is true for DIBuilder, then I'd suggest the same change. If DIBuilder works differently, I'd like to know how to better use it. =]
Compared to an IRBuilder it looks as if DIBuilder has to initialize quite a few members (that will never be used). But I'm unsure whether there actually is a cost to initializing DenseMaps and SmallVectors? Otherwise I'd be happy to instantiate the DIBuilder on the fly. Alternatively we could factor out a less capable DIBuilder base class that does not have all these members.
  
```
    Module &M;
    LLVMContext &VMContext;

    MDNode *TempEnumTypes;
    MDNode *TempRetainTypes;
    MDNode *TempSubprograms;
    MDNode *TempGVs;
    MDNode *TempImportedModules;

    Function *DeclareFn;     // llvm.dbg.declare
    Function *ValueFn;       // llvm.dbg.value

    SmallVector<Metadata *, 4> AllEnumTypes;
    /// Track the RetainTypes, since they can be updated later on.
    SmallVector<TrackingMDNodeRef, 4> AllRetainTypes;
    SmallVector<Metadata *, 4> AllSubprograms;
    SmallVector<Metadata *, 4> AllGVs;
    SmallVector<TrackingMDNodeRef, 4> AllImportedModules;

    /// \brief Track nodes that may be unresolved.
    SmallVector<TrackingMDNodeRef, 4> UnresolvedNodes;
    bool AllowUnresolvedNodes;

    /// Each subprogram's preserved local variables.
    DenseMap<MDNode *, std::vector<TrackingMDNodeRef>> PreservedVariables;
```

================
Comment at: test/DebugInfo/X86/sroasplit-1.ll:3
@@ +2,3 @@
+;
+; rdar://problem/15928306
+;
----------------
chandlerc wrote:
> Honestly, I really dislike seeing rdar links even in tests. They add no information for any reader not at Apple, and so you end up needing to explain all of the context of the test anyways. PR links are at least a reasonable thing for your average contributor to go read.
I can understand that. Removed.

http://reviews.llvm.org/D2680

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/