[PATCH] Debug info: Support fragmented variables.

[Adrian Prantl]

================
Comment at: lib/Transforms/Scalar/SROA.cpp:2119
@@ -2113,1 +2118,3 @@
+
+  DenseMap<AllocaInst *, DbgDeclareInst *>& DbgDeclares;
 
----------------
chandlerc wrote:
> You don't need this -- the SROA pass object is available directly as "Pass".
Thanks!

================
Comment at: lib/Transforms/Scalar/SROA.cpp:2179-2190
@@ -2168,1 +2178,14 @@
 
+    // Migrate debug information from the old alloca to the new alloca
+    // and the individial slices.
+    if (DbgDeclareInst *DbgDecl = DbgDeclares.lookup(&OldAI)) {
+      DIVariable Var(DbgDecl->getVariable());
+      // Create a variable piece to describe the slice. This works fine
+      // even if Var already was a piece.
+      DIExpression Piece = DIB.createPieceExpression(BeginOffset, SliceSize);
+      Instruction *NewDDI = DIB.insertDeclare(&NewAI, Var, Piece, OldPtr);
+      NewDDI->setDebugLoc(DbgDecl->getDebugLoc());
+      DbgDeclares.insert(std::make_pair(&NewAI, cast<DbgDeclareInst>(NewDDI)));
+      Pass.DeadInsts.insert(DbgDecl);
+    }
+
----------------
chandlerc wrote:
> Actually, why do this here at all? This doesn't seem to really be related to rewriting the uses in the traditional sense. Notably, it seems excessive if OldAI == NewAI (which happens in some cases).
> 
> I think the better place to put this logic is inside the rewritePartition method of the SROA pass itself right after we create the new alloca.
Done.

================
Comment at: lib/Transforms/Scalar/SROA.cpp:3725-3726
@@ +3724,4 @@
+    else if (DbgDeclareInst *DDI = dyn_cast<DbgDeclareInst>(I))
+      if (isa<AllocaInst>(DDI->getAddress()))
+        DbgDeclares.insert(std::make_pair(cast<AllocaInst>(DDI->getAddress()),
+                                          DDI));
----------------
chandlerc wrote:
> This should be a dyn_cast to AllocaInst and a re-use.
> 
> Also, getAddress can return null which seems like it would crash here. Unless the verifier rejects such an debug declare inst (and if it does, it would be nice to have an "unsafe" version of the method that returns null for the verifier and have this method assert for us that the result is non-null) this should handle the null case gracefully.
> 
> 
> Somewhat separately (and apologies if this just is my ignorance of debug info) what cases would you expect where the address isn't directly an alloca inst?
See the first testcase:
  define i32 @foo(%struct.Outer* byval align 8 %outer) #0 {
    call void @llvm.dbg.declare(metadata %struct.Outer* %outer, metadata !25, metadata !2), !dbg !26

it could be an argument.

http://reviews.llvm.org/D2680

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