Index: include/llvm/IR/Instruction.h =================================================================== --- include/llvm/IR/Instruction.h +++ include/llvm/IR/Instruction.h @@ -586,6 +586,14 @@ static_cast(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( + static_cast(this)->getPrevNonDebugInstruction()); + } + /// Create a copy of 'this' instruction that is identical in all ways except /// the following: /// * The instruction has no parent Index: lib/IR/Instruction.cpp =================================================================== --- lib/IR/Instruction.cpp +++ lib/IR/Instruction.cpp @@ -602,6 +602,13 @@ return nullptr; } +const Instruction *Instruction::getPrevNonDebugInstruction() const { + for (const Instruction *I = getPrevNode(); I; I = I->getPrevNode()) + if (!isa(I)) + return I; + return nullptr; +} + bool Instruction::isAssociative() const { unsigned Opcode = getOpcode(); if (isAssociative(Opcode)) Index: lib/Transforms/Utils/SimplifyCFG.cpp =================================================================== --- lib/Transforms/Utils/SimplifyCFG.cpp +++ lib/Transforms/Utils/SimplifyCFG.cpp @@ -1372,6 +1372,14 @@ } } + // 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 @@ 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 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 Index: test/CodeGen/X86/pr39187-g.ll =================================================================== --- test/CodeGen/X86/pr39187-g.ll +++ 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)