[llvm] ec7b9b0 - [InstCombine] Avoid redundant or out-of-order debug value sinking

Stephen Tozer via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 26 05:13:57 PST 2021


Author: Stephen Tozer
Date: 2021-02-26T13:04:33Z
New Revision: ec7b9b0c18bb9c9669231bdfa71b0aa7a71140c1

URL: https://github.com/llvm/llvm-project/commit/ec7b9b0c18bb9c9669231bdfa71b0aa7a71140c1
DIFF: https://github.com/llvm/llvm-project/commit/ec7b9b0c18bb9c9669231bdfa71b0aa7a71140c1.diff

LOG: [InstCombine] Avoid redundant or out-of-order debug value sinking

This patch modifies TryToSinkInstruction in the InstCombine pass, to prevent
redundant debug intrinsics from being produced, and also prevent the intrinsics
from being emitted in an incorrect order. It does this by ensuring that when
this pass sinks an instruction and creates clones of the debug intrinsics that
use that instruction, it inserts those debug intrinsics in their original order,
and only inserts the last debug intrinsic for each variable in the Instruction's
block.

Differential revision: https://reviews.llvm.org/D95463

Added: 
    

Modified: 
    llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
    llvm/test/Transforms/InstCombine/debuginfo-sink.ll
    llvm/test/Transforms/InstCombine/debuginfo_add.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index cc16e03c01c6..61dd38783212 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -3577,6 +3577,14 @@ static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
   // here, but that computation has been sunk.
   SmallVector<DbgVariableIntrinsic *, 2> DbgUsers;
   findDbgUsers(DbgUsers, I);
+  // Process the sinking DbgUsers in reverse order, as we only want to clone the
+  // last appearing debug intrinsic for each given variable.
+  SmallVector<DbgVariableIntrinsic *, 2> DbgUsersToSink;
+  for (DbgVariableIntrinsic *DVI : DbgUsers)
+    if (DVI->getParent() == SrcBlock)
+      DbgUsersToSink.push_back(DVI);
+  llvm::sort(DbgUsersToSink,
+             [](auto *A, auto *B) { return B->comesBefore(A); });
 
   // Update the arguments of a dbg.declare instruction, so that it
   // does not point into a sunk instruction.
@@ -3592,12 +3600,20 @@ static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
   };
 
   SmallVector<DbgVariableIntrinsic *, 2> DIIClones;
-  for (auto User : DbgUsers) {
+  SmallSet<DebugVariable, 4> SunkVariables;
+  for (auto User : DbgUsersToSink) {
     // A dbg.declare instruction should not be cloned, since there can only be
     // one per variable fragment. It should be left in the original place
     // because the sunk instruction is not an alloca (otherwise we could not be
     // here).
-    if (User->getParent() != SrcBlock || updateDbgDeclare(User))
+    if (updateDbgDeclare(User))
+      continue;
+
+    DebugVariable DbgUserVariable =
+        DebugVariable(User->getVariable(), User->getExpression(),
+                      User->getDebugLoc()->getInlinedAt());
+
+    if (!SunkVariables.insert(DbgUserVariable).second)
       continue;
 
     DIIClones.emplace_back(cast<DbgVariableIntrinsic>(User->clone()));
@@ -3607,7 +3623,9 @@ static bool TryToSinkInstruction(Instruction *I, BasicBlock *DestBlock) {
   // Perform salvaging without the clones, then sink the clones.
   if (!DIIClones.empty()) {
     salvageDebugInfoForDbgValues(*I, DbgUsers);
-    for (auto &DIIClone : DIIClones) {
+    // The clones are in reverse order of original appearance, reverse again to
+    // maintain the original order.
+    for (auto &DIIClone : llvm::reverse(DIIClones)) {
       DIIClone->insertBefore(&*InsertPos);
       LLVM_DEBUG(dbgs() << "SINK: " << *DIIClone << '\n');
     }

diff  --git a/llvm/test/Transforms/InstCombine/debuginfo-sink.ll b/llvm/test/Transforms/InstCombine/debuginfo-sink.ll
index 1e6277748fae..5a8cc78b08c2 100644
--- a/llvm/test/Transforms/InstCombine/debuginfo-sink.ll
+++ b/llvm/test/Transforms/InstCombine/debuginfo-sink.ll
@@ -52,6 +52,33 @@ sink2:
   ret i32 %0
 }
 
+; This GEP is sunk, and has multiple debug uses in the same block. Check that
+; only the last use is cloned into the sunk block, and that both of the
+; original dbg.values are salvaged.
+;
+; CHECK-LABEL: define i32 @baz(i32*
+; CHECK:       call void @llvm.dbg.value(metadata i32* %a, metadata !{{[0-9]+}},
+; CHECK-SAME:  metadata !DIExpression(DW_OP_plus_uconst, 4, DW_OP_stack_value))
+; CHECK-NEXT:  call void @llvm.dbg.value(metadata i32* %a, metadata !{{[0-9]+}},
+; CHECK-SAME:  metadata !DIExpression(DW_OP_plus_uconst, 4, DW_OP_plus_uconst, 5, DW_OP_stack_value))
+; CHECK-NEXT:  br label %sink1
+
+define i32 @baz(i32 *%a) !dbg !80 {
+entry:
+  %gep = getelementptr i32, i32 *%a, i32 1
+  call void @llvm.dbg.value(metadata i32 *%gep, metadata !83, metadata !12), !dbg !84
+  call void @llvm.dbg.value(metadata i32 *%gep, metadata !83, metadata !DIExpression(DW_OP_plus_uconst, 5)), !dbg !85
+  br label %sink1
+
+sink1:
+; CHECK-LABEL: sink1:
+; CHECK:       call void @llvm.dbg.value(metadata i32* %gep,
+; CHECK-SAME:  metadata !{{[0-9]+}}, metadata !DIExpression(DW_OP_plus_uconst, 5))
+; CHECK-NEXT:  load
+  %0 = load i32, i32* %gep, align 4, !dbg !85
+  ret i32 %0, !dbg !85
+}
+
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4, !5}
 !llvm.ident = !{!6}
@@ -76,3 +103,7 @@ sink2:
 !72 = !{!10, !10, !10}
 !73 = !DILocalVariable(name: "k", scope: !70, file: !1, line: 2, type: !10)
 !74 = !DILocation(line: 5, column: 3, scope: !70)
+!80 = distinct !DISubprogram(name: "baz", scope: !1, file: !1, line: 2, type: !8, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: false, unit: !0, retainedNodes: !2)
+!83 = !DILocalVariable(name: "l", scope: !80, file: !1, line: 2, type: !10)
+!84 = !DILocation(line: 5, column: 3, scope: !80)
+!85 = !DILocation(line: 6, column: 3, scope: !80)

diff  --git a/llvm/test/Transforms/InstCombine/debuginfo_add.ll b/llvm/test/Transforms/InstCombine/debuginfo_add.ll
index ae83e3a95382..fa21c19bb6a0 100644
--- a/llvm/test/Transforms/InstCombine/debuginfo_add.ll
+++ b/llvm/test/Transforms/InstCombine/debuginfo_add.ll
@@ -36,8 +36,8 @@ for.body.lr.ph:                                   ; preds = %entry
   ; The add is later eliminated, so we verify that the dbg.value is salvaged by using DW_OP_minus.
   ; CHECK-LABEL: for.body.lr.ph:
   ; CHECK-NEXT: %0 = load
-  ; CHECK-NEXT: call void @llvm.dbg.value(metadata i64 %0, metadata !26, metadata !DIExpression(DW_OP_constu, 4096, DW_OP_minus, DW_OP_stack_value)), !dbg !
   ; CHECK-NEXT: call void @llvm.dbg.value(metadata i64 %0, metadata !25, metadata !DIExpression()), !dbg !
+  ; CHECK-NEXT: call void @llvm.dbg.value(metadata i64 %0, metadata !26, metadata !DIExpression(DW_OP_constu, 4096, DW_OP_minus, DW_OP_stack_value)), !dbg !
   br label %for.body, !dbg !32
 
 for.body:                                         ; preds = %for.body.lr.ph, %for.body


        


More information about the llvm-commits mailing list