Skip to content

Commit 06a2128

Browse files
taewookohtaewookoh
committed
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
1 parent 19f2403 commit 06a2128

File tree

3 files changed

+116
-2
lines changed

3 files changed

+116
-2
lines changed

‎llvm/include/llvm/CodeGen/MachineBasicBlock.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,10 @@ class MachineBasicBlock
664664
return findDebugLoc(MBBI.getInstrIterator());
665665
}
666666

667+
/// Find and return the merged DebugLoc of the branch instructions of the
668+
/// block. Return UnknownLoc if there is none.
669+
DebugLoc findBranchDebugLoc();
670+
667671
/// Possible outcome of a register liveness query to computeRegisterLiveness()
668672
enum LivenessQueryResult {
669673
LQR_Live, ///< Register is known to be (at least partially) live.

‎llvm/lib/CodeGen/MachineBasicBlock.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include "llvm/CodeGen/SlotIndexes.h"
2424
#include "llvm/IR/BasicBlock.h"
2525
#include "llvm/IR/DataLayout.h"
26+
#include "llvm/IR/DebugInfoMetadata.h"
2627
#include "llvm/IR/ModuleSlotTracker.h"
2728
#include "llvm/MC/MCAsmInfo.h"
2829
#include "llvm/MC/MCContext.h"
@@ -423,7 +424,7 @@ void MachineBasicBlock::updateTerminator() {
423424

424425
MachineBasicBlock *TBB = nullptr, *FBB = nullptr;
425426
SmallVector<MachineOperand, 4> Cond;
426-
DebugLoc DL; // FIXME: this is nowhere
427+
DebugLoc DL = findBranchDebugLoc();
427428
bool B = TII->analyzeBranch(*this, TBB, FBB, Cond);
428429
(void) B;
429430
assert(!B && "UpdateTerminators requires analyzable predecessors!");
@@ -491,7 +492,7 @@ void MachineBasicBlock::updateTerminator() {
491492
// FIXME: This does not seem like a reasonable pattern to support, but it
492493
// has been seen in the wild coming out of degenerate ARM test cases.
493494
TII->removeBranch(*this);
494-
495+
495496
// Finally update the unconditional successor to be reached via a branch if
496497
// it would not be reached by fallthrough.
497498
if (!isLayoutSuccessor(TBB))
@@ -1150,6 +1151,24 @@ MachineBasicBlock::findDebugLoc(instr_iterator MBBI) {
11501151
return {};
11511152
}
11521153

1154+
/// Find and return the merged DebugLoc of the branch instructions of the block.
1155+
/// Return UnknownLoc if there is none.
1156+
DebugLoc
1157+
MachineBasicBlock::findBranchDebugLoc() {
1158+
DebugLoc DL {};
1159+
auto TI = getFirstTerminator();
1160+
while (TI != end() && !TI->isBranch())
1161+
++TI;
1162+
1163+
if (TI != end()) {
1164+
DL = TI->getDebugLoc();
1165+
for (++TI ; TI != end() ; ++TI)
1166+
if (TI->isBranch())
1167+
DL = DILocation::getMergedLocation(DL, TI->getDebugLoc());
1168+
}
1169+
return DL;
1170+
}
1171+
11531172
/// Return probability of the edge from this block to MBB.
11541173
BranchProbability
11551174
MachineBasicBlock::getSuccProbability(const_succ_iterator Succ) const {

‎llvm/test/CodeGen/X86/update-terminator-debugloc.ll

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
; RUN: llc -stop-after=machine-sink -march=x86-64 < %s | FileCheck %s
2+
;
3+
; test code:
4+
; 1 extern int bar(int x);
5+
; 2
6+
; 3 int foo(int *begin, int *end) {
7+
; 4 int *i;
8+
; 5 int ret = 0;
9+
; 6 for (
10+
; 7 i = begin ;
11+
; 8 i != end ;
12+
; 9 i++)
13+
; 10 {
14+
; 11 ret += bar(*i);
15+
; 12 }
16+
; 13 return ret;
17+
; 14 }
18+
;
19+
; With the test code, LLVM-IR below shows that loop-control branches have a
20+
; debug location of line 6 (branches in entry and for.body block). Make sure that
21+
; these debug locations are propaged correctly to lowered instructions.
22+
;
23+
; CHECK: [[DLOC:![0-9]+]] = !DILocation(line: 6
24+
; CHECK-DAG: [[VREG1:%[^ ]+]] = COPY %rsi
25+
; CHECK-DAG: [[VREG2:%[^ ]+]] = COPY %rdi
26+
; CHECK: SUB64rr [[VREG2]], [[VREG1]]
27+
; CHECK-NEXT: JNE_1 {{.*}}, debug-location [[DLOC]]{{$}}
28+
; CHECK: [[VREG3:%[^ ]+]] = PHI [[VREG2]]
29+
; CHECK: [[VREG4:%[^ ]+]] = ADD64ri8 [[VREG3]], 4
30+
; CHECK: SUB64rr [[VREG1]], [[VREG4]]
31+
; CHECK-NEXT: JNE_1 {{.*}}, debug-location [[DLOC]]{{$}}
32+
; CHECK-NEXT: JMP_1 {{.*}}, debug-location [[DLOC]]{{$}}
33+
34+
target triple = "x86_64-unknown-linux-gnu"
35+
36+
define i32 @foo(i32* readonly %begin, i32* readnone %end) !dbg !4 {
37+
entry:
38+
%cmp6 = icmp eq i32* %begin, %end, !dbg !9
39+
br i1 %cmp6, label %for.end, label %for.body.preheader, !dbg !12
40+
41+
for.body.preheader: ; preds = %entry
42+
br label %for.body, !dbg !13
43+
44+
for.body: ; preds = %for.body.preheader, %for.body
45+
%ret.08 = phi i32 [ %add, %for.body ], [ 0, %for.body.preheader ]
46+
%i.07 = phi i32* [ %incdec.ptr, %for.body ], [ %begin, %for.body.preheader ]
47+
%0 = load i32, i32* %i.07, align 4, !dbg !13, !tbaa !15
48+
%call = tail call i32 @bar(i32 %0), !dbg !19
49+
%add = add nsw i32 %call, %ret.08, !dbg !20
50+
%incdec.ptr = getelementptr inbounds i32, i32* %i.07, i64 1, !dbg !21
51+
%cmp = icmp eq i32* %incdec.ptr, %end, !dbg !9
52+
br i1 %cmp, label %for.end.loopexit, label %for.body, !dbg !12, !llvm.loop !22
53+
54+
for.end.loopexit: ; preds = %for.body
55+
br label %for.end, !dbg !24
56+
57+
for.end: ; preds = %for.end.loopexit, %entry
58+
%ret.0.lcssa = phi i32 [ 0, %entry ], [ %add, %for.end.loopexit ]
59+
ret i32 %ret.0.lcssa, !dbg !24
60+
}
61+
62+
declare i32 @bar(i32)
63+
64+
!llvm.dbg.cu = !{!0}
65+
!llvm.module.flags = !{!2, !3}
66+
67+
!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1)
68+
!1 = !DIFile(filename: "foo.c", directory: "b/")
69+
!2 = !{i32 2, !"Dwarf Version", i32 4}
70+
!3 = !{i32 2, !"Debug Info Version", i32 3}
71+
!4 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 3, type: !5, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !0)
72+
!5 = !DISubroutineType(types: !6)
73+
!6 = !{!7, !8, !8}
74+
!7 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
75+
!8 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !7, size: 64)
76+
!9 = !DILocation(line: 8, column: 9, scope: !10)
77+
!10 = distinct !DILexicalBlock(scope: !11, file: !1, line: 6, column: 3)
78+
!11 = distinct !DILexicalBlock(scope: !4, file: !1, line: 6, column: 3)
79+
!12 = !DILocation(line: 6, column: 3, scope: !11)
80+
!13 = !DILocation(line: 11, column: 18, scope: !14)
81+
!14 = distinct !DILexicalBlock(scope: !10, file: !1, line: 10, column: 3)
82+
!15 = !{!16, !16, i64 0}
83+
!16 = !{!"int", !17, i64 0}
84+
!17 = !{!"omnipotent char", !18, i64 0}
85+
!18 = !{!"Simple C/C++ TBAA"}
86+
!19 = !DILocation(line: 11, column: 14, scope: !14)
87+
!20 = !DILocation(line: 11, column: 11, scope: !14)
88+
!21 = !DILocation(line: 9, column: 8, scope: !10)
89+
!22 = distinct !{!22, !12, !23}
90+
!23 = !DILocation(line: 12, column: 3, scope: !11)
91+
!24 = !DILocation(line: 13, column: 3, scope: !4)

0 commit comments

Comments
 (0)