Skip to content

Commit

Permalink
[DebugInfo] Give inlinable calls DILocs (PR39807)
Browse files Browse the repository at this point in the history
In PR39807 we incorrectly handle circumstances where calls are common'd
from conditional blocks into the parent BB. Calls that can be inlined
must always have DebugLocs, however we strip them during commoning, which
the IR verifier asserts on.

Fix this by using applyMergedLocation: it will perform the same DebugLoc
stripping of conditional Locs, but will also generate an unknown location
DebugLoc that satisfies the requirement for inlinable calls to always have
locations.

Some of the prior logic for selecting a DebugLoc is now likely redundant;
I'll generate a follow-up to remove it (involves editing more regression
tests).

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

llvm-svn: 347782
  • Loading branch information
jmorse committed Nov 28, 2018
1 parent 63d397e commit 9b4cfa5
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 8 deletions.
17 changes: 9 additions & 8 deletions llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Expand Up @@ -1375,7 +1375,7 @@ 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
// provide the debug location for generated select instructions. For BBs
// with only debug instructions, use an empty debug location.
Instruction *InsertPt =
BIParent->getTerminator()->getPrevNonDebugInstruction();
Expand All @@ -1389,15 +1389,16 @@ 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());
// Ensure terminator gets a debug location, even an unknown one, in case
// it involves inlinable calls.
NT->applyMergedLocation(I1->getDebugLoc(), I2->getDebugLoc());

IRBuilder<NoFolder> Builder(NT);
// If an earlier instruction in this BB had a location, adopt it, otherwise
// clear debug locations.
Builder.SetCurrentDebugLocation(InsertPt ? InsertPt->getDebugLoc()
: DebugLoc());

// 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
// them. If they do, all PHI entries for BB1/BB2 must agree for all PHI
Expand Down
43 changes: 43 additions & 0 deletions llvm/test/Transforms/SimplifyCFG/pr39807.ll
@@ -0,0 +1,43 @@
; RUN: opt -S -simplifycfg < %s | FileCheck %s

declare void @personality()

define void @test(i1 %b) personality void()* @personality !dbg !1 {
; CHECK: invoke void @inlinable()
; CHECK-NEXT: to label %success unwind label %failure, !dbg ![[DBGLOC:[0-9]+]]
br i1 %b, label %if, label %else

if:
invoke void @inlinable()
to label %success unwind label %failure, !dbg !2

else:
invoke void @inlinable()
to label %success unwind label %failure, !dbg !8

success:
ret void

failure:
landingpad {}
cleanup
ret void
}

define internal void @inlinable() !dbg !7 {
ret void
}

!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!4, !5, !6}

; CHECK: ![[DBGLOC]] = !DILocation(line: 0
!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, runtimeVersion: 0, file: !3)
!1 = distinct !DISubprogram(name: "test", unit: !0)
!2 = !DILocation(line: 2, scope: !1)
!3 = !DIFile(filename: "foo", directory: ".")
!4 = !{i32 2, !"Dwarf Version", i32 4}
!5 = !{i32 2, !"Debug Info Version", i32 3}
!6 = !{i32 1, !"wchar_size", i32 4}
!7 = distinct !DISubprogram(name: "inlinable", unit: !0)
!8 = !DILocation(line: 3, scope: !1)

0 comments on commit 9b4cfa5

Please sign in to comment.