Skip to content

Commit

Permalink
Remove debug location from common tail when tail-merging
Browse files Browse the repository at this point in the history
The branch folding pass tail merges blocks into a common-tail.  However, the
tail retains the debug information from one of the original inputs to the
merge (chosen randomly).  This is a problem for sampled-based PGO, as hits
on the common-tail will be attributed to whichever block was chosen,
irrespective of which path was actually taken to the common-tail.

This patch fixes the issue by nulling the debug location for the common-tail.

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

llvm-svn: 285093
  • Loading branch information
Robert Lougher committed Oct 25, 2016
1 parent cffedc4 commit e325647
Show file tree
Hide file tree
Showing 3 changed files with 85 additions and 8 deletions.
7 changes: 5 additions & 2 deletions llvm/lib/CodeGen/BranchFolding.cpp
Expand Up @@ -720,8 +720,6 @@ bool BranchFolder::CreateCommonTailOnlyBlock(MachineBasicBlock *&PredBB,
SameTails[commonTailIndex].getTailStartPos();
MachineBasicBlock *MBB = SameTails[commonTailIndex].getBlock();

// If the common tail includes any debug info we will take it pretty
// randomly from one of the inputs. Might be better to remove it?
DEBUG(dbgs() << "\nSplitting BB#" << MBB->getNumber() << ", size "
<< maxCommonTailLength);

Expand Down Expand Up @@ -898,6 +896,11 @@ bool BranchFolder::TryTailMergeBlocks(MachineBasicBlock *SuccBB,
// Recompute common tail MBB's edge weights and block frequency.
setCommonTailEdgeWeights(*MBB);

// Remove the original debug location from the common tail.
for (auto &MI : *MBB)
if (!MI.isDebugValue())
MI.setDebugLoc(DebugLoc());

// MBB is common tail. Adjust all other BB's to jump to this one.
// Traversal must be forwards so erases work.
DEBUG(dbgs() << "\nUsing common tail in BB#" << MBB->getNumber()
Expand Down
10 changes: 4 additions & 6 deletions llvm/test/DebugInfo/COFF/local-variables.ll
Expand Up @@ -63,11 +63,9 @@
; ASM: .cv_loc 2 1 5 3 # t.cpp:5:3
; ASM: callq capture
; ASM: leaq 36(%rsp), %rcx
; ASM: [[inline_site2_end:\.Ltmp.*]]:
; ASM: [[else_end:\.Ltmp.*]]:
; ASM: .LBB0_3: # %if.end
; ASM: .cv_loc 0 1 15 5 # t.cpp:15:5
; ASM: callq capture
; ASM: [[else_end:\.Ltmp.*]]:
; ASM: .cv_loc 0 1 17 1 # t.cpp:17:1
; ASM: nop
; ASM: addq $56, %rsp
Expand Down Expand Up @@ -101,7 +99,7 @@
; ASM: .long 116 # TypeIndex
; ASM: .short 0 # Flags
; ASM: .asciz "v"
; ASM: .cv_def_range [[inline_site2]] [[inline_site2_end]], "E\021O\001\000\0000\000\000\000"
; ASM: .cv_def_range [[inline_site2]] [[else_end]], "E\021O\001\000\0000\000\000\000"
; ASM: .short 4430 # Record kind: S_INLINESITE_END

; OBJ: Subsection [
Expand Down Expand Up @@ -159,7 +157,7 @@
; OBJ: LocalVariableAddrRange {
; OBJ: OffsetStart: .text+0x2D
; OBJ: ISectStart: 0x0
; OBJ: Range: 0x24
; OBJ: Range: 0x1F
; OBJ: }
; OBJ: }
; OBJ: InlineSite {
Expand Down Expand Up @@ -200,7 +198,7 @@
; OBJ: ChangeLineOffset: 1
; OBJ: ChangeCodeOffset: 0x35
; OBJ: ChangeCodeOffsetAndLineOffset: {CodeOffset: 0xD, LineOffset: 1}
; OBJ: ChangeCodeLength: 0xA
; OBJ: ChangeCodeLength: 0xF
; OBJ: ]
; OBJ: }
; OBJ: Local {
Expand Down
76 changes: 76 additions & 0 deletions llvm/test/DebugInfo/X86/tail-merge.ll
@@ -0,0 +1,76 @@
; RUN: llc %s -mtriple=x86_64-unknown-unknown -use-unknown-locations=true -o - | FileCheck %s

; Generated with "clang -gline-tables-only -c -emit-llvm -o - | opt -sroa -S"
; from source:
;
; extern int foo(int);
; extern int bar(int);
;
; int test(int a, int b) {
; if(b)
; a += foo(a);
; else
; a += bar(a);
; return a;
; }

; When tail-merging the debug location of the common tail should be removed.

; CHECK-LABEL: test:
; CHECK: movl %edi, [[REG:%.*]]
; CHECK: testl %esi, %esi
; CHECK: je [[ELSE:.LBB[0-9]+_[0-9]+]]
; CHECK: .loc 1 6 10
; CHECK: callq foo
; CHECK: jmp [[TAIL:.LBB[0-9]+_[0-9]+]]
; CHECK: [[ELSE]]:
; CHECK: .loc 1 8 10
; CHECK: callq bar
; CHECK: [[TAIL]]:
; CHECK: .loc 1 0 0
; CHECK: addl [[REG]], %eax
; CHECK: .loc 1 9 3

target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define i32 @test(i32 %a, i32 %b) !dbg !6 {
entry:
%tobool = icmp ne i32 %b, 0, !dbg !8
br i1 %tobool, label %if.then, label %if.else, !dbg !8

if.then: ; preds = %entry
%call = call i32 @foo(i32 %a), !dbg !9
%add = add nsw i32 %a, %call, !dbg !10
br label %if.end, !dbg !11

if.else: ; preds = %entry
%call1 = call i32 @bar(i32 %a), !dbg !12
%add2 = add nsw i32 %a, %call1, !dbg !13
br label %if.end

if.end: ; preds = %if.else, %if.then
%a.addr.0 = phi i32 [ %add, %if.then ], [ %add2, %if.else ]
ret i32 %a.addr.0, !dbg !14
}

declare i32 @foo(i32)
declare i32 @bar(i32)

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

!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "", isOptimized: false, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
!1 = !DIFile(filename: "test.c", directory: "")
!2 = !{}
!3 = !{i32 2, !"Dwarf Version", i32 4}
!4 = !{i32 2, !"Debug Info Version", i32 3}
!6 = distinct !DISubprogram(name: "test", scope: !1, file: !1, line: 4, type: !7, isLocal: false, isDefinition: true, scopeLine: 4, flags: DIFlagPrototyped, isOptimized: false, unit: !0, variables: !2)
!7 = !DISubroutineType(types: !2)
!8 = !DILocation(line: 5, column: 6, scope: !6)
!9 = !DILocation(line: 6, column: 10, scope: !6)
!10 = !DILocation(line: 6, column: 7, scope: !6)
!11 = !DILocation(line: 6, column: 5, scope: !6)
!12 = !DILocation(line: 8, column: 10, scope: !6)
!13 = !DILocation(line: 8, column: 7, scope: !6)
!14 = !DILocation(line: 9, column: 3, scope: !6)

0 comments on commit e325647

Please sign in to comment.