[SimplifyCFG] Update debug location when folding branch to common destination

[davide]

Summary

Fixes https://bugs.llvm.org/show_bug.cgi?id=46008

SimplifyCFG flattens the CFG transforming:

@a = local_unnamed_addr global i16 1, align 2, !dbg !0
@c = local_unnamed_addr global i32 0, align 4, !dbg !9
@b = local_unnamed_addr global i32 0, align 4, !dbg !6

; Function Attrs: nounwind ssp uwtable
define void @d() #0 !dbg !17 {
  ret void, !dbg !20
}

; Function Attrs: nounwind ssp uwtable
define i32 @main() local_unnamed_addr #0 !dbg !21 {
  call void @llvm.dbg.value(metadata i32* @c, metadata !25, metadata !DIExpression()), !dbg !27
  br label %1, !dbg !28

1:                                                ; preds = %7, %0
  %2 = load i16, i16* @a, align 2, !dbg !29, !tbaa !32
  %3 = icmp slt i16 %2, 1, !dbg !36
  br i1 %3, label %4, label %8, !dbg !37

4:                                                ; preds = %1
  %5 = load i32, i32* @c, align 4, !dbg !38, !tbaa !40
  %6 = icmp eq i32 %5, 0, !dbg !42
  br i1 %6, label %7, label %8, !dbg !43

7:                                                ; preds = %4
  store i16 ptrtoint (void ()* @d to i16), i16* @a, align 2, !dbg !44, !tbaa !32
  br label %1, !dbg !45, !llvm.loop !46

8:                                                ; preds = %4, %1
  %9 = load i32, i32* @c, align 4, !dbg !49, !tbaa !40
  store i32 %9, i32* @b, align 4, !dbg !50, !tbaa !40
  ret i32 0, !dbg !51
}

into

; Function Attrs: nounwind ssp uwtable
define i32 @main() local_unnamed_addr #0 !dbg !21 {
  call void @llvm.dbg.value(metadata i32* @c, metadata !25, metadata !DIExpression()), !dbg !27
  br label %1, !dbg !28

1:                                                ; preds = %6, %0
  %2 = load i16, i16* @a, align 2, !dbg !29, !tbaa !32
  %3 = icmp slt i16 %2, 1, !dbg !36
  %4 = load i32, i32* @c, align 4, !dbg !37
  %5 = icmp eq i32 %4, 0, !dbg !39
  %or.cond = and i1 %3, %5, !dbg !40
  br i1 %or.cond, label %6, label %7, !dbg !40

6:                                                ; preds = %1
  store i16 ptrtoint (void ()* @d to i16), i16* @a, align 2, !dbg !41, !tbaa !32
  br label %1, !dbg !42, !llvm.loop !43

7:                                                ; preds = %1
  %8 = load i32, i32* @c, align 4, !dbg !46, !tbaa !47
  store i32 %8, i32* @b, align 4, !dbg !49, !tbaa !47
  ret i32 0, !dbg !50
}

When the instructions get folded, a dead block gets folded and the debug information is still retained. This manifests as jumpy stepping in lldb, see the bugzilla PR for an end-to-end C testcase

[davide]

Correct diff.

[vsk]

Thanks, lgtm!

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

2642

Ah - In the single predecessor case, we erase instructions from the successor and replace their uses with an equivalent instruction in the predecessor. So I think this is fine, no change needed here.

2761

Ditto, it seems fine to keep the location of the condition on the 'not' instruction.

[davide]

commit 8cdd2a158c9cc94ad265fe4f0d45edd3bf916b6f (HEAD -> master, origin/master, origin/HEAD)
Author: Davide Italiano <[e-mail]>
Date: Thu Jun 18 12:33:02 2020 -0700

[SimplifyCFG] Update debug location when folding branch to common destination

Sometimes a dead block gets folded and the debug information is still
retained. This manifests as jumpy stepping in lldb, see the bugzilla PR
for an end-to-end C testcase.

Fixes https://bugs.llvm.org/show_bug.cgi?id=46008

Differential Revision:  https://reviews.llvm.org/D82062

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

if (BasicBlock *PB = BB->getSinglePredecessor())
                     E = BB->instructionsWithoutDebug().end();
                I != E;) {
             Instruction *Curr = &*I++;
             if (isa<CmpInst>(Curr)) {
               Cond = Curr;
               break;
             }
             // Quit if we can't remove this instruction.
             if (!tryCSEWithPredecessor(Curr, PB))
               return false;
             Changed = true;
           }
         }
 
     if (!Cond)
       return Changed;
   }
for (pred_iterator PI = pred_begin(BB), E = pred_end(BB); PI != E; ++PI) {
     if (InvertPredCond) {
       Value *NewCond = PBI->getCondition();
 
       if (NewCond->hasOneUse() && isa<CmpInst>(NewCond)) {
         CmpInst *CI = cast<CmpInst>(NewCond);
         CI->setPredicate(CI->getInversePredicate());
       } else {
         NewCond =
             Builder.CreateNot(NewCond, PBI->getCondition()->getName() + ".not");
       }
 
       PBI->setCondition(NewCond);
       PBI->swapSuccessors();
     }
 
     // If we have bonus instructions, clone them into the predecessor block.
     // Note that there may be multiple predecessor blocks, so we cannot move
     // bonus instructions to a predecessor block.
     ValueToValueMapTy VMap; // maps original values to cloned values
     // We already make sure Cond is the last instruction before BI. Therefore,
     // all instructions before Cond other than DbgInfoIntrinsic are bonus
     // instructions.
     for (auto BonusInst = BB->begin(); Cond != &*BonusInst; ++BonusInst) {
       if (isa<DbgInfoIntrinsic>(BonusInst))
         continue;
       Instruction *NewBonusInst = BonusInst->clone();
+
+      // When we fold the bonus instructions we want to make sure we
+      // reset their debug locations in order to avoid stepping on dead
+      // code caused by folding dead branches.
+      NewBonusInst->setDebugLoc(DebugLoc());
+
       RemapInstruction(NewBonusInst, VMap,
                        RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
       VMap[&*BonusInst] = NewBonusInst;
 
       // If we moved a load, we cannot any longer claim any knowledge about
       // its potential value. The previous information might have been valid
       // only given the branch precondition.
       // For an analogous reason, we must also drop all the metadata whose
       // semantics we don't understand.
       NewBonusInst->dropUnknownNonDebugMetadata();
 
       PredBlock->getInstList().insert(PBI->getIterator(), NewBonusInst);
       NewBonusInst->takeName(&*BonusInst);
       BonusInst->setName(BonusInst->getName() + ".old");
     }
 
     // Clone Cond into the predecessor basic block, and or/and the
     // two conditions together.
     Instruction *CondInPred = Cond->clone();
+
+    // Reset the condition debug location to avoid jumping on dead code
+    // as the result of folding dead branches.
+    CondInPred->setDebugLoc(DebugLoc());
+
     RemapInstruction(CondInPred, VMap,
                      RF_NoModuleLevelChanges | RF_IgnoreMissingLocals);
     PredBlock->getInstList().insert(PBI->getIterator(), CondInPred);
     CondInPred->takeName(Cond);
     Cond->setName(CondInPred->getName() + ".old");
 
     if (BI->isConditional()) {
       Instruction *NewCond = cast<Instruction>(

llvm/test/Transforms/SimplifyCFG/basictest.ll

 
 
 ; PR14893
 define i8 @test6f() {
 ; CHECK-LABEL: @test6f
 ; CHECK: alloca i8, align 1
 ; CHECK-NEXT: call i8 @test6g
 ; CHECK-NEXT: icmp eq i8 %tmp, 0
-; CHECK-NEXT: load i8, i8* %r, align 1, !dbg !{{[0-9]+$}}
+; CHECK-NEXT: load i8, i8* %r, align 1{{$}}
 
 bb0:
   %r = alloca i8, align 1
   %tmp = call i8 @test6g(i8* %r)
   %tmp1 = icmp eq i8 %tmp, 0
   br i1 %tmp1, label %bb2, label %bb1
 bb1:
   %tmp3 = load i8, i8* %r, align 1, !range !2, !tbaa !10, !dbg !5

llvm/test/Transforms/SimplifyCFG/fold-debug-location.ll

+; RUN: opt -S -simplifycfg < %s | FileCheck %s --match-full-lines
+
+; Make sure we reset the debug location when folding instructions.
+; CHECK: [[VAL:%.*]] = and i32 %c2, %k
+; CHECK-NEXT: [[VAL2:%.*]] icmp eq i32 [[VAL]], 0
+
+declare i32 @bar(...)
+
+define i32 @patatino(i32 %k, i32 %c1, i32 %c2) !dbg !6 {
+  %1 = and i32 %c1, %k, !dbg !8
+  %2 = icmp eq i32 %1, 0, !dbg !9
+  br i1 %2, label %8, label %3, !dbg !10
+
+3:
+  %4 = and i32 %c2, %k, !dbg !11
+  %5 = icmp eq i32 %4, 0, !dbg !12
+  br i1 %5, label %8, label %6, !dbg !13
+
+6:
+  %7 = tail call i32 (...) @bar(), !dbg !14
+  br label %8, !dbg !15
+
+8:
+  ret i32 undef, !dbg !16
+}
+
+!llvm.dbg.cu = !{!0}
+!llvm.debugify = !{!3, !4}
+!llvm.module.flags = !{!5}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C, file: !1, producer: "debugify", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
+!1 = !DIFile(filename: "a.ll", directory: "/")
+!2 = !{}
+!3 = !{i32 9}
+!4 = !{i32 0}
+!5 = !{i32 2, !"Debug Info Version", i32 3}
+!6 = distinct !DISubprogram(name: "patatino", linkageName: "patatino", scope: null, file: !1, line: 1, type: !7, scopeLine: 1, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !2)
+!7 = !DISubroutineType(types: !2)
+!8 = !DILocation(line: 1, column: 1, scope: !6)
+!9 = !DILocation(line: 2, column: 1, scope: !6)
+!10 = !DILocation(line: 3, column: 1, scope: !6)
+!11 = !DILocation(line: 4, column: 1, scope: !6)
+!12 = !DILocation(line: 5, column: 1, scope: !6)
+!13 = !DILocation(line: 6, column: 1, scope: !6)
+!14 = !DILocation(line: 7, column: 1, scope: !6)
+!15 = !DILocation(line: 8, column: 1, scope: !6)
+!16 = !DILocation(line: 9, column: 1, scope: !6)