Skip to content
This repository has been archived by the owner on Feb 5, 2019. It is now read-only.

Commit

Permalink
[DebugInfo][Dexter] Unreachable line stepped onto after SimplifyCFG.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
CarlosAlbertoEnciso committed Nov 9, 2018
1 parent 8e070ff commit 8381d3e
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 0 deletions.
8 changes: 8 additions & 0 deletions include/llvm/IR/Instruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,14 @@ class Instruction : public User,
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
Expand Down
7 changes: 7 additions & 0 deletions lib/IR/Instruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
16 changes: 16 additions & 0 deletions lib/Transforms/Utils/SimplifyCFG.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1372,6 +1372,14 @@ static bool HoistThenElseCodeToIf(BranchInst *BI,
}
}

// 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);
Expand All @@ -1381,6 +1389,14 @@ static bool HoistThenElseCodeToIf(BranchInst *BI,
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
Expand Down
121 changes: 121 additions & 0 deletions test/CodeGen/X86/pr39187-g.ll
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 8381d3e

Please sign in to comment.