diff options
author | Taewook Oh <twoh@fb.com> | 2017-02-13 18:15:31 +0000 |
---|---|---|
committer | Taewook Oh <twoh@fb.com> | 2017-02-13 18:15:31 +0000 |
commit | 06a2128cfa56e0d4954d231b86f44fe04ae8db95 (patch) | |
tree | a97850057641565352fc3cdde5681a0fb6b8f5bd /llvm/lib/CodeGen/MachineBasicBlock.cpp | |
parent | 19f2403d3ba89a394e7146fd239ecab4ceb773db (diff) | |
download | llvm-06a2128cfa56e0d4954d231b86f44fe04ae8db95.zip llvm-06a2128cfa56e0d4954d231b86f44fe04ae8db95.tar.gz llvm-06a2128cfa56e0d4954d231b86f44fe04ae8db95.tar.bz2 |
Make MachineBasicBlock::updateTerminator to update DebugLoc as well
Summary:
Currently MachineBasicBlock::updateTerminator simply drops DebugLoc for newly created branch instructions, which may cause incorrect stepping and/or imprecise sample profile data. Below is an example:
```
1 extern int bar(int x);
2
3 int foo(int *begin, int *end) {
4 int *i;
5 int ret = 0;
6 for (
7 i = begin ;
8 i != end ;
9 i++)
10 {
11 ret += bar(*i);
12 }
13 return ret;
14 }
```
Below is a bitcode of 'foo' at the end of LLVM-IR level optimizations with -O3:
```
define i32 @foo(i32* readonly %begin, i32* readnone %end) !dbg !4 {
entry:
%cmp6 = icmp eq i32* %begin, %end, !dbg !9
br i1 %cmp6, label %for.end, label %for.body.preheader, !dbg !12
for.body.preheader: ; preds = %entry
br label %for.body, !dbg !13
for.body: ; preds = %for.body.preheader, %for.body
%ret.08 = phi i32 [ %add, %for.body ], [ 0, %for.body.preheader ]
%i.07 = phi i32* [ %incdec.ptr, %for.body ], [ %begin, %for.body.preheader ]
%0 = load i32, i32* %i.07, align 4, !dbg !13, !tbaa !15
%call = tail call i32 @bar(i32 %0), !dbg !19
%add = add nsw i32 %call, %ret.08, !dbg !20
%incdec.ptr = getelementptr inbounds i32, i32* %i.07, i64 1, !dbg !21
%cmp = icmp eq i32* %incdec.ptr, %end, !dbg !9
br i1 %cmp, label %for.end.loopexit, label %for.body, !dbg !12, !llvm.loop !22
for.end.loopexit: ; preds = %for.body
br label %for.end, !dbg !24
for.end: ; preds = %for.end.loopexit, %entry
%ret.0.lcssa = phi i32 [ 0, %entry ], [ %add, %for.end.loopexit ]
ret i32 %ret.0.lcssa, !dbg !24
}
```
where
```
!12 = !DILocation(line: 6, column: 3, scope: !11)
```
. As you can see, the terminator of 'entry' block, which is a loop control branch, has a DebugLoc of line 6, column 3. Howerver, after the execution of 'MachineBlock::updateTerminator' function, which is triggered by MachineSinking pass, the DebugLoc info is dropped as below (see there's no debug-location for JNE_1):
```
bb.0.entry:
successors: %bb.4(0x30000000), %bb.1.for.body.preheader(0x50000000)
liveins: %rdi, %rsi
%6 = COPY %rsi
%5 = COPY %rdi
%8 = SUB64rr %5, %6, implicit-def %eflags, debug-location !9
JNE_1 %bb.1.for.body.preheader, implicit %eflags
```
This patch addresses this issue and make newly created branch instructions to keep debug-location info.
Reviewers: aprantl, MatzeB, craig.topper, qcolombet
Reviewed By: qcolombet
Subscribers: qcolombet, llvm-commits
Differential Revision: https://reviews.llvm.org/D29596
llvm-svn: 294976
Diffstat (limited to 'llvm/lib/CodeGen/MachineBasicBlock.cpp')
-rw-r--r-- | llvm/lib/CodeGen/MachineBasicBlock.cpp | 23 |
1 files changed, 21 insertions, 2 deletions
diff --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp index a7d87799..2468bfa 100644 --- a/llvm/lib/CodeGen/MachineBasicBlock.cpp +++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp @@ -23,6 +23,7 @@ #include "llvm/CodeGen/SlotIndexes.h" #include "llvm/IR/BasicBlock.h" #include "llvm/IR/DataLayout.h" +#include "llvm/IR/DebugInfoMetadata.h" #include "llvm/IR/ModuleSlotTracker.h" #include "llvm/MC/MCAsmInfo.h" #include "llvm/MC/MCContext.h" @@ -423,7 +424,7 @@ void MachineBasicBlock::updateTerminator() { MachineBasicBlock *TBB = nullptr, *FBB = nullptr; SmallVector<MachineOperand, 4> Cond; - DebugLoc DL; // FIXME: this is nowhere + DebugLoc DL = findBranchDebugLoc(); bool B = TII->analyzeBranch(*this, TBB, FBB, Cond); (void) B; assert(!B && "UpdateTerminators requires analyzable predecessors!"); @@ -491,7 +492,7 @@ void MachineBasicBlock::updateTerminator() { // FIXME: This does not seem like a reasonable pattern to support, but it // has been seen in the wild coming out of degenerate ARM test cases. TII->removeBranch(*this); - + // Finally update the unconditional successor to be reached via a branch if // it would not be reached by fallthrough. if (!isLayoutSuccessor(TBB)) @@ -1150,6 +1151,24 @@ MachineBasicBlock::findDebugLoc(instr_iterator MBBI) { return {}; } +/// Find and return the merged DebugLoc of the branch instructions of the block. +/// Return UnknownLoc if there is none. +DebugLoc +MachineBasicBlock::findBranchDebugLoc() { + DebugLoc DL {}; + auto TI = getFirstTerminator(); + while (TI != end() && !TI->isBranch()) + ++TI; + + if (TI != end()) { + DL = TI->getDebugLoc(); + for (++TI ; TI != end() ; ++TI) + if (TI->isBranch()) + DL = DILocation::getMergedLocation(DL, TI->getDebugLoc()); + } + return DL; +} + /// Return probability of the edge from this block to MBB. BranchProbability MachineBasicBlock::getSuccProbability(const_succ_iterator Succ) const { |