[PATCH] Debug info: Support fragmented variables.
Chandler Carruth
chandlerc at gmail.com
Mon Mar 24 12:08:25 PDT 2014
Mostly commenting on the SROA things.
================
Comment at: lib/Transforms/Scalar/SROA.cpp:2116-2117
@@ +2115,4 @@
+ unsigned VarSize = Var.getSizeInBits(TypeIdentifierMap) / 8;
+ assert(BeginOffset+SliceSize <= VarSize &&
+ "slice is larger than or outside of variable");
+ DIVariable Piece = (SliceSize == VarSize)
----------------
This being an assert worries me. Does the verifier ensure this?
================
Comment at: lib/Transforms/Scalar/SROA.cpp:2124
@@ +2123,3 @@
+ DbgDeclares.insert(std::make_pair(&NewAI,cast<DbgDeclareInst>(NewDDI)));
+ Pass.DeadInsts.insert(DbgDecl);
+ }
----------------
This doesn't make sense to me. We may see the same DbgDecl many times? Wouldn't this become dead only when OldAI became dead?
================
Comment at: lib/Transforms/Scalar/SROA.cpp:970-972
@@ -968,1 +969,5 @@
+ /// Debug intrinsics do not show up like regular uses in the
+ /// IR. This side-table holds the missing use edges.
+ DenseMap<Value *, DbgDeclareInst *> DbgDeclares;
+
----------------
If these are actually indexed by AllocaInst*s, why not use that as the key type?
================
Comment at: lib/Transforms/Scalar/SROA.cpp:975-976
@@ +974,4 @@
+ /// Holds a map from DITypeRef to the actual MDNode.
+ DITypeIdentifierMapCache TypeIdentifierMapCache;
+ Optional<DITypeIdentifierMap> TypeIdentifierMap;
+
----------------
This is a pretty terrible interface... Why not just lazily create this and store it in the Module so that the Module provides a direct method to produce a TypeIdentifierMap?
Also, why Optional rather than a pointer? I assume TypeIdentifierMap is some kind of smart pointer?
================
Comment at: lib/Transforms/Scalar/SROA.cpp:2107-2108
@@ -2089,1 +2106,4 @@
+ if (&NewAI != &OldAI) {
+ // Migrate debug information to the new alloca.
+ if (DbgDeclareInst *DbgDecl = DbgDeclares.lookup(&OldAI)) {
----------------
I don't think this belongs in the slice rewriter. The rewriter is about rewriting uses in the slice vector.
There is a perfect place for this in the rewritePartition method right after we create the new AllocaInst.
http://llvm-reviews.chandlerc.com/D2680
More information about the llvm-commits
mailing list