[mem2reg] Remove dbg.values describing contents of dead allocas

OCHyams · OCHyams · commit fea067bdfde4 · 2020-10-23T04:46:56.000Z
This patch copies @vSK's fix to instcombine from D85555 over to mem2reg. The motivation and rationale are exactly the same: When mem2reg removes an alloca, it erases the dbg.{addr,declare} instructions which refer to the alloca. It would be better to instead remove all debug intrinsics which describe the contents of the dead alloca, namely all dbg.value(<dead alloca>, ..., DW_OP_deref)'s. As far as I can tell, prior to D80264 these `dbg.value+deref`s would have been silently dropped instead of being made `undef`, so we're just returning to previous behaviour with these patches. Testing: `llvm-lit llvm/test` and `ninja check-clang` gave no unexpected failures. Added 3 tests, each of which covers a dbg.value deletion path in mem2reg: mem2reg-promote-alloca-1.ll mem2reg-promote-alloca-2.ll mem2reg-promote-alloca-3.ll The first is based on the dexter test inlining.c from D89543. This patch also improves the debugging experience for loop.c from D89543, which suffers similarly after arg promotion instead of inlining.
diff --git a/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp b/llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp
@@ -101,22 +101,24 @@ bool llvm::isAllocaPromotable(const AllocaInst *AI) {
 namespace {
 
 struct AllocaInfo {
+  using DbgUserVec = SmallVector<DbgVariableIntrinsic *, 1>;
+
   SmallVector<BasicBlock *, 32> DefiningBlocks;
   SmallVector<BasicBlock *, 32> UsingBlocks;
 
   StoreInst *OnlyStore;
   BasicBlock *OnlyBlock;
   bool OnlyUsedInOneBlock;
 
-  TinyPtrVector<DbgVariableIntrinsic *> DbgDeclares;
+  DbgUserVec DbgUsers;
 
   void clear() {
     DefiningBlocks.clear();
     UsingBlocks.clear();
     OnlyStore = nullptr;
     OnlyBlock = nullptr;
     OnlyUsedInOneBlock = true;
-    DbgDeclares.clear();
+    DbgUsers.clear();
   }
 
   /// Scan the uses of the specified alloca, filling in the AllocaInfo used
@@ -149,7 +151,7 @@ struct AllocaInfo {
       }
     }
 
-    DbgDeclares = FindDbgAddrUses(AI);
+    findDbgUsers(DbgUsers, AI);
   }
 };
 
@@ -247,7 +249,7 @@ struct PromoteMem2Reg {
   /// For each alloca, we keep track of the dbg.declare intrinsic that
   /// describes it, if any, so that we can convert it to a dbg.value
   /// intrinsic if the alloca gets promoted.
-  SmallVector<TinyPtrVector<DbgVariableIntrinsic *>, 8> AllocaDbgDeclares;
+  SmallVector<AllocaInfo::DbgUserVec, 8> AllocaDbgUsers;
 
   /// The set of basic blocks the renamer has already visited.
   SmallPtrSet<BasicBlock *, 16> Visited;
@@ -421,10 +423,14 @@ static bool rewriteSingleStoreAlloca(AllocaInst *AI, AllocaInfo &Info,
 
   // Record debuginfo for the store and remove the declaration's
   // debuginfo.
-  for (DbgVariableIntrinsic *DII : Info.DbgDeclares) {
-    DIBuilder DIB(*AI->getModule(), /*AllowUnresolved*/ false);
-    ConvertDebugDeclareToDebugValue(DII, Info.OnlyStore, DIB);
-    DII->eraseFromParent();
+  for (DbgVariableIntrinsic *DII : Info.DbgUsers) {
+    if (DII->isAddressOfVariable()) {
+      DIBuilder DIB(*AI->getModule(), /*AllowUnresolved*/ false);
+      ConvertDebugDeclareToDebugValue(DII, Info.OnlyStore, DIB);
+      DII->eraseFromParent();
+    } else if (DII->getExpression()->startsWithDeref()) {
+      DII->eraseFromParent();
+    }
   }
   // Remove the (now dead) store and alloca.
   Info.OnlyStore->eraseFromParent();
@@ -519,9 +525,11 @@ static bool promoteSingleBlockAlloca(AllocaInst *AI, const AllocaInfo &Info,
   while (!AI->use_empty()) {
     StoreInst *SI = cast<StoreInst>(AI->user_back());
     // Record debuginfo for the store before removing it.
-    for (DbgVariableIntrinsic *DII : Info.DbgDeclares) {
-      DIBuilder DIB(*AI->getModule(), /*AllowUnresolved*/ false);
-      ConvertDebugDeclareToDebugValue(DII, SI, DIB);
+    for (DbgVariableIntrinsic *DII : Info.DbgUsers) {
+      if (DII->isAddressOfVariable()) {
+        DIBuilder DIB(*AI->getModule(), /*AllowUnresolved*/ false);
+        ConvertDebugDeclareToDebugValue(DII, SI, DIB);
+      }
     }
     SI->eraseFromParent();
     LBI.deleteValue(SI);
@@ -530,8 +538,9 @@ static bool promoteSingleBlockAlloca(AllocaInst *AI, const AllocaInfo &Info,
   AI->eraseFromParent();
 
   // The alloca's debuginfo can be removed as well.
-  for (DbgVariableIntrinsic *DII : Info.DbgDeclares)
-    DII->eraseFromParent();
+  for (DbgVariableIntrinsic *DII : Info.DbgUsers)
+    if (DII->isAddressOfVariable() || DII->getExpression()->startsWithDeref())
+      DII->eraseFromParent();
 
   ++NumLocalPromoted;
   return true;
@@ -540,7 +549,7 @@ static bool promoteSingleBlockAlloca(AllocaInst *AI, const AllocaInfo &Info,
 void PromoteMem2Reg::run() {
   Function &F = *DT.getRoot()->getParent();
 
-  AllocaDbgDeclares.resize(Allocas.size());
+  AllocaDbgUsers.resize(Allocas.size());
 
   AllocaInfo Info;
   LargeBlockInfo LBI;
@@ -598,8 +607,8 @@ void PromoteMem2Reg::run() {
     }
 
     // Remember the dbg.declare intrinsic describing this alloca, if any.
-    if (!Info.DbgDeclares.empty())
-      AllocaDbgDeclares[AllocaNum] = Info.DbgDeclares;
+    if (!Info.DbgUsers.empty())
+      AllocaDbgUsers[AllocaNum] = Info.DbgUsers;
 
     // Keep the reverse mapping of the 'Allocas' array for the rename pass.
     AllocaLookup[Allocas[AllocaNum]] = AllocaNum;
@@ -672,9 +681,11 @@ void PromoteMem2Reg::run() {
   }
 
   // Remove alloca's dbg.declare instrinsics from the function.
-  for (auto &Declares : AllocaDbgDeclares)
-    for (auto *DII : Declares)
-      DII->eraseFromParent();
+  for (auto &DbgUsers : AllocaDbgUsers) {
+    for (auto *DII : DbgUsers)
+      if (DII->isAddressOfVariable() || DII->getExpression()->startsWithDeref())
+        DII->eraseFromParent();
+  }
 
   // Loop over all of the PHI nodes and see if there are any that we can get
   // rid of because they merge all of the same incoming values.  This can
@@ -914,8 +925,9 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
 
         // The currently active variable for this block is now the PHI.
         IncomingVals[AllocaNo] = APN;
-        for (DbgVariableIntrinsic *DII : AllocaDbgDeclares[AllocaNo])
-          ConvertDebugDeclareToDebugValue(DII, APN, DIB);
+        for (DbgVariableIntrinsic *DII : AllocaDbgUsers[AllocaNo])
+          if (DII->isAddressOfVariable())
+            ConvertDebugDeclareToDebugValue(DII, APN, DIB);
 
         // Get the next phi node.
         ++PNI;
@@ -974,8 +986,9 @@ void PromoteMem2Reg::RenamePass(BasicBlock *BB, BasicBlock *Pred,
 
       // Record debuginfo for the store before removing it.
       IncomingLocs[AllocaNo] = SI->getDebugLoc();
-      for (DbgVariableIntrinsic *DII : AllocaDbgDeclares[ai->second])
-        ConvertDebugDeclareToDebugValue(DII, SI, DIB);
+      for (DbgVariableIntrinsic *DII : AllocaDbgUsers[ai->second])
+        if (DII->isAddressOfVariable())
+          ConvertDebugDeclareToDebugValue(DII, SI, DIB);
       BB->getInstList().erase(SI);
     }
   }
diff --git a/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-1.ll b/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-1.ll
@@ -0,0 +1,77 @@
+; RUN: opt -mem2reg %s -S -o - | FileCheck %s
+
+;; Check that mem2reg removes dbg.value(%param.addr, DIExpression(DW_OP_deref...))
+;; when promoting the alloca %param.addr.
+;;
+;; $ clang inlining.c -O2 -g -emit-llvm -S -o tmp.ll -Xclang -disable-llvm-passes
+;; $ opt tmp.ll -o - -instcombine -inline -S
+;; $ cat inlining.c
+;; int g;
+;; __attribute__((__always_inline__))
+;; static void use(int* p) {
+;;   g = *p;
+;; }
+;;
+;; __attribute__((__noinline__))
+;; void fun(int param) {
+;;   use(&param);
+;; }
+
+; CHECK: define dso_local void @fun(i32 %param)
+; CHECK-NEXT: entry:
+; CHECK-NEXT: call void @llvm.dbg.value(metadata i32 %param, metadata ![[PARAM:[0-9]+]], metadata !DIExpression())
+; CHECK-NOT: call void @llvm.dbg.value({{.*}}, metadata ![[PARAM]]
+; CHECK: ![[PARAM]] = !DILocalVariable(name: "param",
+
+@g = dso_local global i32 0, align 4, !dbg !0
+
+define dso_local void @fun(i32 %param) !dbg !12 {
+entry:
+  %param.addr = alloca i32, align 4
+  call void @llvm.dbg.value(metadata i32 %param, metadata !16, metadata !DIExpression()), !dbg !17
+  store i32 %param, i32* %param.addr, align 4
+  call void @llvm.dbg.value(metadata i32* %param.addr, metadata !16, metadata !DIExpression(DW_OP_deref)), !dbg !17
+  call void @llvm.dbg.value(metadata i32* %param.addr, metadata !22, metadata !DIExpression()), !dbg !28
+  call void @llvm.dbg.value(metadata i32* %param.addr, metadata !22, metadata !DIExpression()), !dbg !28
+  %0 = load i32, i32* %param.addr, align 4, !dbg !30
+  store i32 %0, i32* @g, align 4, !dbg !31
+  ret void, !dbg !32
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!8, !9, !10}
+!llvm.ident = !{!11}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "g", scope: !2, file: !6, line: 8, type: !7, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 12.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "inlining.c", directory: "/")
+!4 = !{}
+!5 = !{!0}
+!6 = !DIFile(filename: "inlining.c", directory: "/")
+!7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!8 = !{i32 7, !"Dwarf Version", i32 4}
+!9 = !{i32 2, !"Debug Info Version", i32 3}
+!10 = !{i32 1, !"wchar_size", i32 4}
+!11 = !{!"clang version 12.0.0"}
+!12 = distinct !DISubprogram(name: "fun", scope: !6, file: !6, line: 15, type: !13, scopeLine: 15, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !15)
+!13 = !DISubroutineType(types: !14)
+!14 = !{null, !7}
+!15 = !{!16}
+!16 = !DILocalVariable(name: "param", arg: 1, scope: !12, file: !6, line: 15, type: !7)
+!17 = !DILocation(line: 0, scope: !12)
+!22 = !DILocalVariable(name: "p", arg: 1, scope: !23, file: !6, line: 10, type: !26)
+!23 = distinct !DISubprogram(name: "use", scope: !6, file: !6, line: 10, type: !24, scopeLine: 10, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !27)
+!24 = !DISubroutineType(types: !25)
+!25 = !{null, !26}
+!26 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 64)
+!27 = !{!22}
+!28 = !DILocation(line: 0, scope: !23, inlinedAt: !29)
+!29 = distinct !DILocation(line: 16, column: 3, scope: !12)
+!30 = !DILocation(line: 11, column: 7, scope: !23, inlinedAt: !29)
+!31 = !DILocation(line: 11, column: 5, scope: !23, inlinedAt: !29)
+!32 = !DILocation(line: 17, column: 1, scope: !12)
+!34 = !DISubroutineType(types: !35)
+!35 = !{!7}
diff --git a/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-2.ll b/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-2.ll
@@ -0,0 +1,124 @@
+; RUN: opt -mem2reg %s -S -o - | FileCheck %s
+
+;; Check that mem2reg removes dbg.value(%local, DIExpression(DW_OP_deref...))
+;; that instcombine LowerDbgDeclare inserted before the call to 'esc' when
+;; promoting the alloca %local after 'esc' has been inlined. Without this we
+;; provide no location for 'local', even though it is provably constant
+;; throughout after inlining.
+;;
+;; $ clang reduce.c -O2 -g -emit-llvm -S -o tmp.ll -Xclang -disable-llvm-passes
+;; $ opt tmp.ll -o - -instcombine -inline -S
+;; $ cat reduce.c
+;; long a;
+;; int b;
+;; void c();
+;; __attribute__((__always_inline__))
+;; static void esc(long *e) {
+;;   *e = a;
+;;   c();
+;;   if (b)
+;;     *e = 0;
+;; }
+;;
+;; void fun() {
+;;   long local = 0;
+;;   esc(&local);
+;; }
+
+; CHECK: define dso_local void @fun()
+; CHECK-NEXT: entry:
+; CHECK-NEXT: call void @llvm.dbg.value(metadata i64 0, metadata ![[LOCAL:[0-9]+]], metadata !DIExpression())
+; CHECK-NOT: call void @llvm.dbg.value({{.*}}, metadata ![[LOCAL]]
+; CHECK: ![[LOCAL]] = !DILocalVariable(name: "local",
+
+@a = dso_local global i64 0, align 8, !dbg !0
+@b = dso_local global i32 0, align 4, !dbg !6
+
+define dso_local void @fun() !dbg !14 {
+entry:
+  %e.addr.i = alloca i64*, align 8
+  %local = alloca i64, align 8
+  %0 = bitcast i64* %local to i8*, !dbg !19
+  call void @llvm.lifetime.start.p0i8(i64 8, i8* nonnull %0), !dbg !19
+  call void @llvm.dbg.value(metadata i64 0, metadata !18, metadata !DIExpression()), !dbg !20
+  store i64 0, i64* %local, align 8, !dbg !21
+  call void @llvm.dbg.value(metadata i64* %local, metadata !18, metadata !DIExpression(DW_OP_deref)), !dbg !20
+  %1 = bitcast i64** %e.addr.i to i8*, !dbg !26
+  call void @llvm.lifetime.start.p0i8(i64 8, i8* %1), !dbg !26
+  call void @llvm.dbg.value(metadata i64* %local, metadata !32, metadata !DIExpression()), !dbg !26
+  store i64* %local, i64** %e.addr.i, align 8
+  %2 = load i64, i64* @a, align 8, !dbg !36
+  call void @llvm.dbg.value(metadata i64* %local, metadata !32, metadata !DIExpression()), !dbg !26
+  store i64 %2, i64* %local, align 8, !dbg !37
+  call void (...) @c(), !dbg !38
+  %3 = load i32, i32* @b, align 4, !dbg !39
+  %tobool.not.i = icmp eq i32 %3, 0, !dbg !39
+  br i1 %tobool.not.i, label %esc.exit, label %if.then.i, !dbg !43
+
+if.then.i:                                        ; preds = %entry
+  %4 = load i64*, i64** %e.addr.i, align 8, !dbg !44
+  call void @llvm.dbg.value(metadata i64* %4, metadata !32, metadata !DIExpression()), !dbg !26
+  store i64 0, i64* %4, align 8, !dbg !45
+  br label %esc.exit, !dbg !46
+
+esc.exit:                                         ; preds = %entry, %if.then.i
+  %5 = bitcast i64** %e.addr.i to i8*, !dbg !47
+  call void @llvm.lifetime.end.p0i8(i64 8, i8* %5), !dbg !47
+  %6 = bitcast i64* %local to i8*, !dbg !48
+  call void @llvm.lifetime.end.p0i8(i64 8, i8* nonnull %6), !dbg !48
+  ret void, !dbg !48
+}
+
+declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture)
+declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture)
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+declare !dbg !49 dso_local void @c(...)
+
+!llvm.dbg.cu = !{!2}
+!llvm.module.flags = !{!10, !11, !12}
+!llvm.ident = !{!13}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "a", scope: !2, file: !3, line: 1, type: !9, isLocal: false, isDefinition: true)
+!2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !3, producer: "clang version 12.0.0)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !5, splitDebugInlining: false, nameTableKind: None)
+!3 = !DIFile(filename: "reduce.c", directory: "/")
+!4 = !{}
+!5 = !{!0, !6}
+!6 = !DIGlobalVariableExpression(var: !7, expr: !DIExpression())
+!7 = distinct !DIGlobalVariable(name: "b", scope: !2, file: !3, line: 2, type: !8, isLocal: false, isDefinition: true)
+!8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!9 = !DIBasicType(name: "long int", size: 64, encoding: DW_ATE_signed)
+!10 = !{i32 7, !"Dwarf Version", i32 4}
+!11 = !{i32 2, !"Debug Info Version", i32 3}
+!12 = !{i32 1, !"wchar_size", i32 4}
+!13 = !{!"clang version 12.0.0"}
+!14 = distinct !DISubprogram(name: "fun", scope: !3, file: !3, line: 12, type: !15, scopeLine: 12, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !17)
+!15 = !DISubroutineType(types: !16)
+!16 = !{null}
+!17 = !{!18}
+!18 = !DILocalVariable(name: "local", scope: !14, file: !3, line: 13, type: !9)
+!19 = !DILocation(line: 13, column: 3, scope: !14)
+!20 = !DILocation(line: 0, scope: !14)
+!21 = !DILocation(line: 13, column: 8, scope: !14)
+!26 = !DILocation(line: 0, scope: !27, inlinedAt: !33)
+!27 = distinct !DISubprogram(name: "esc", scope: !3, file: !3, line: 5, type: !28, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !31)
+!28 = !DISubroutineType(types: !29)
+!29 = !{null, !30}
+!30 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !9, size: 64)
+!31 = !{!32}
+!32 = !DILocalVariable(name: "e", arg: 1, scope: !27, file: !3, line: 5, type: !30)
+!33 = distinct !DILocation(line: 14, column: 3, scope: !14)
+!36 = !DILocation(line: 6, column: 8, scope: !27, inlinedAt: !33)
+!37 = !DILocation(line: 6, column: 6, scope: !27, inlinedAt: !33)
+!38 = !DILocation(line: 7, column: 3, scope: !27, inlinedAt: !33)
+!39 = !DILocation(line: 8, column: 7, scope: !40, inlinedAt: !33)
+!40 = distinct !DILexicalBlock(scope: !27, file: !3, line: 8, column: 7)
+!43 = !DILocation(line: 8, column: 7, scope: !27, inlinedAt: !33)
+!44 = !DILocation(line: 9, column: 6, scope: !40, inlinedAt: !33)
+!45 = !DILocation(line: 9, column: 8, scope: !40, inlinedAt: !33)
+!46 = !DILocation(line: 9, column: 5, scope: !40, inlinedAt: !33)
+!47 = !DILocation(line: 10, column: 1, scope: !27, inlinedAt: !33)
+!48 = !DILocation(line: 15, column: 1, scope: !14)
+!49 = !DISubprogram(name: "c", scope: !3, file: !3, line: 3, type: !50, spFlags: DISPFlagOptimized, retainedNodes: !4)
+!50 = !DISubroutineType(types: !51)
+!51 = !{null, null}
diff --git a/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-3.ll b/llvm/test/DebugInfo/Generic/mem2reg-promote-alloca-3.ll
