[llvm] r346481 - [DebugInfo][Dexter] Unreachable line stepped onto after SimplifyCFG.
Carlos Alberto Enciso via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 9 01:42:11 PST 2018
Author: carlos.alberto.enciso
Date: Fri Nov 9 01:42:10 2018
New Revision: 346481
URL: http://llvm.org/viewvc/llvm-project?rev=346481&view=rev
Log:
[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
Added:
llvm/trunk/test/CodeGen/X86/pr39187-g.ll
Modified:
llvm/trunk/include/llvm/IR/Instruction.h
llvm/trunk/lib/IR/Instruction.cpp
llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
Modified: llvm/trunk/include/llvm/IR/Instruction.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Instruction.h?rev=346481&r1=346480&r2=346481&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Instruction.h (original)
+++ llvm/trunk/include/llvm/IR/Instruction.h Fri Nov 9 01:42:10 2018
@@ -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
Modified: llvm/trunk/lib/IR/Instruction.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Instruction.cpp?rev=346481&r1=346480&r2=346481&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Instruction.cpp (original)
+++ llvm/trunk/lib/IR/Instruction.cpp Fri Nov 9 01:42:10 2018
@@ -602,6 +602,13 @@ const Instruction *Instruction::getNextN
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))
Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=346481&r1=346480&r2=346481&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Fri Nov 9 01:42:10 2018
@@ -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
Added: llvm/trunk/test/CodeGen/X86/pr39187-g.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/pr39187-g.ll?rev=346481&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/pr39187-g.ll (added)
+++ llvm/trunk/test/CodeGen/X86/pr39187-g.ll Fri Nov 9 01:42:10 2018
@@ -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)
More information about the llvm-commits
mailing list