[DebugInfo][Dexter] Unreachable line stepped onto after SimplifyCFG.

In SimplifyCFG when given a conditional branch that goes to BB1 and BB2, the hoisted common terminator instruction in the two blocks, caused debug line records associated with subsequent select instructions to become ambiguous. It causes the debugger to display unreachable source lines.

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@346481 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/IR/Instruction.h b/include/llvm/IR/Instruction.h
index 2a75801..e1a1fae 100644 (file)
--- a/include/llvm/IR/Instruction.h
+++ b/include/llvm/IR/Instruction.h
@@ -586,6 +586,14 @@ public:
         static_cast<const Instruction *>(this)->getNextNonDebugInstruction());
   }
 
+  /// Return a pointer to the previous non-debug instruction in the same basic
+  /// block as 'this', or nullptr if no such instruction exists.
+  const Instruction *getPrevNonDebugInstruction() const;
+  Instruction *getPrevNonDebugInstruction() {
+    return const_cast<Instruction *>(
+        static_cast<const Instruction *>(this)->getPrevNonDebugInstruction());
+  }
+
   /// Create a copy of 'this' instruction that is identical in all ways except
   /// the following:
   ///   * The instruction has no parent

diff --git a/lib/IR/Instruction.cpp b/lib/IR/Instruction.cpp
index 75e7413..1e32637 100644 (file)
--- a/lib/IR/Instruction.cpp
+++ b/lib/IR/Instruction.cpp
@@ -602,6 +602,13 @@ const Instruction *Instruction::getNextNonDebugInstruction() const {
   return nullptr;
 }
 
+const Instruction *Instruction::getPrevNonDebugInstruction() const {
+  for (const Instruction *I = getPrevNode(); I; I = I->getPrevNode())
+    if (!isa<DbgInfoIntrinsic>(I))
+      return I;
+  return nullptr;
+}
+
 bool Instruction::isAssociative() const {
   unsigned Opcode = getOpcode();
   if (isAssociative(Opcode))

diff --git a/lib/Transforms/Utils/SimplifyCFG.cpp b/lib/Transforms/Utils/SimplifyCFG.cpp
index 849f9ee..af9d2ea 100644 (file)
--- a/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1372,6 +1372,14 @@ HoistTerminator:
     }
   }
 
+  // As the parent basic block terminator is a branch instruction which is
+  // removed at the end of the current transformation, use its previous
+  // non-debug instruction, as the reference insertion point, which will
+  // provide the debug location for the instruction being hoisted. For BBs
+  // with only debug instructions, use an empty debug location.
+  Instruction *InsertPt =
+      BIParent->getTerminator()->getPrevNonDebugInstruction();
+
   // Okay, it is safe to hoist the terminator.
   Instruction *NT = I1->clone();
   BIParent->getInstList().insert(BI->getIterator(), NT);
@@ -1381,6 +1389,14 @@ HoistTerminator:
     NT->takeName(I1);
   }
 
+  // The instruction NT being hoisted, is the terminator for the true branch,
+  // with debug location (DILocation) within that branch. We can't retain
+  // its original debug location value, otherwise 'select' instructions that
+  // are created from any PHI nodes, will take its debug location, giving
+  // the impression that those 'select' instructions are in the true branch,
+  // causing incorrect stepping, affecting the debug experience.
+  NT->setDebugLoc(InsertPt ? InsertPt->getDebugLoc() : DebugLoc());
+
   IRBuilder<NoFolder> Builder(NT);
   // Hoisting one of the terminators from our successor is a great thing.
   // Unfortunately, the successors of the if/else blocks may have PHI nodes in

diff --git a/test/CodeGen/X86/pr39187-g.ll b/test/CodeGen/X86/pr39187-g.ll
new file mode 100644 (file)
index 0000000..46da641
--- /dev/null
+++ b/test/CodeGen/X86/pr39187-g.ll
@@ -0,0 +1,121 @@
+; RUN: opt < %s -S -simplifycfg | FileCheck %s
+
+; SimplifyCFG can hoist any common code in the 'then' and 'else' blocks to
+; the 'if' basic block.
+;
+; For the special case, when hoisting the terminator instruction, its debug
+; location keep references to its basic block, causing the debug information
+; to become ambiguous. It causes the debugger to display unreached lines.
+
+; Change the debug location associated with the hoisted instruction, to
+; the debug location from the insertion point in the 'if' block.
+
+; The insertion point is the previous non-debug instruction before the
+; terminator in the parent basic block of the hoisted instruction.
+
+; IR with '-g':
+;
+;  [...]
+;  %frombool = zext i1 %cmp to i8, !dbg !26
+;  call void @llvm.dbg.value(metadata i8 %frombool, metadata !15, metadata !DIExpression()), !dbg !26
+;  call void @llvm.dbg.value(metadata i32 0, metadata !17, metadata !DIExpression()), !dbg !27
+;  br i1 %cmp, label %if.then, label %if.else
+;  [...]
+;
+; Insertion point is: %frombool = zext i1 %cmp to i8, !dbg !26
+
+; IR generated with:
+; clang -S -g -gno-column-info -O2 -emit-llvm pr39187.cpp -o pr39187-g.ll -mllvm -opt-bisect-limit=10
+
+; // pr39187.cpp
+; int main() {
+;   volatile int foo = 0;
+;
+;   int beards = 0;
+;   bool cond = foo == 4;
+;   int bar = 0;
+;   if (cond)
+;     beards = 8;
+;   else
+;     beards = 4;
+;
+;   volatile bool face = cond;
+;
+;   return face ? beards : 0;
+; }
+
+; CHECK-LABEL: entry
+; CHECK:  %foo = alloca i32, align 4
+; CHECK:  %face = alloca i8, align 1
+; CHECK:  %foo.0..sroa_cast = bitcast i32* %foo to i8*
+; CHECK:  store volatile i32 0, i32* %foo, align 4
+; CHECK:  %foo.0. = load volatile i32, i32* %foo, align 4, !dbg !16
+; CHECK:  %cmp = icmp eq i32 %foo.0., 4, !dbg !16
+; CHECK:  %frombool = zext i1 %cmp to i8, !dbg !16
+; CHECK:  call void @llvm.dbg.value(metadata i8 %frombool, metadata !13, metadata !DIExpression()), !dbg !16
+; CHECK:  call void @llvm.dbg.value(metadata i32 0, metadata !15, metadata !DIExpression()), !dbg !17
+; CHECK:  %. = select i1 %cmp, i32 8, i32 4, !dbg !16
+
+; ModuleID = 'pr39187.cpp'
+source_filename = "pr39187.cpp"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-pc-linux-gnu"
+
+; Function Attrs: norecurse nounwind uwtable
+define dso_local i32 @main() local_unnamed_addr #0 !dbg !7 {
+entry:
+  %foo = alloca i32, align 4
+  %face = alloca i8, align 1
+  %foo.0..sroa_cast = bitcast i32* %foo to i8*
+  store volatile i32 0, i32* %foo, align 4
+  %foo.0. = load volatile i32, i32* %foo, align 4, !dbg !26
+  %cmp = icmp eq i32 %foo.0., 4, !dbg !26
+  %frombool = zext i1 %cmp to i8, !dbg !26
+  call void @llvm.dbg.value(metadata i8 %frombool, metadata !15, metadata !DIExpression()), !dbg !26
+  call void @llvm.dbg.value(metadata i32 0, metadata !17, metadata !DIExpression()), !dbg !27
+  br i1 %cmp, label %if.then, label %if.else
+
+if.then:                                          ; preds = %entry
+  call void @llvm.dbg.value(metadata i32 8, metadata !14, metadata !DIExpression()), !dbg !25
+  br label %if.end
+
+if.else:                                          ; preds = %entry
+  call void @llvm.dbg.value(metadata i32 4, metadata !14, metadata !DIExpression()), !dbg !25
+  br label %if.end
+
+if.end:                                           ; preds = %if.else, %if.then
+  %beards.0 = phi i32 [ 8, %if.then ], [ 4, %if.else ]
+  store volatile i8 %frombool, i8* %face, align 1
+  %face.0. = load volatile i8, i8* %face, align 1
+  %0 = and i8 %face.0., 1
+  %tobool3 = icmp eq i8 %0, 0
+  %cond4 = select i1 %tobool3, i32 0, i32 %beards.0
+  ret i32 %cond4
+}
+
+; Function Attrs: nounwind readnone speculatable
+declare void @llvm.dbg.value(metadata, metadata, metadata) #2
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 8.0.0 (trunk 346301)", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, nameTableKind: None)
+!1 = !DIFile(filename: "pr39187.cpp", directory: ".")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{!"clang version 8.0.0 (trunk 346301)"}
+!7 = distinct !DISubprogram(name: "main", scope: !1, file: !1, line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: true, unit: !0, retainedNodes: !11)
+!8 = !DISubroutineType(types: !9)
+!9 = !{!10}
+!10 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!11 = !{!14, !15, !17}
+!14 = !DILocalVariable(name: "beards", scope: !7, file: !1, line: 4, type: !10)
+!15 = !DILocalVariable(name: "cond", scope: !7, file: !1, line: 5, type: !16)
+!16 = !DIBasicType(name: "bool", size: 8, encoding: DW_ATE_boolean)
+!17 = !DILocalVariable(name: "bar", scope: !7, file: !1, line: 6, type: !10)
+!25 = !DILocation(line: 4, scope: !7)
+!26 = !DILocation(line: 5, scope: !7)
+!27 = !DILocation(line: 6, scope: !7)