[llvm] r285212 - Reapply: "Remove debug location from common tail when tail-merging"

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 31 10:14:56 PDT 2016


On Wed, Oct 26, 2016 at 10:11 AM Robert Lougher via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: rlougher
> Date: Wed Oct 26 12:01:47 2016
> New Revision: 285212
>
> URL: http://llvm.org/viewvc/llvm-project?rev=285212&view=rev
> Log:
> Reapply: "Remove debug location from common tail when tail-merging"
>
> This reapplies revision 285093.


What was the fix that made this commit of the same change work where the
previous one was reverted? (please mention this in future commit messages -
it makes it easier to review the changes, etc)


> Original commit message:
>
> 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
>
> Added:
>     llvm/trunk/test/DebugInfo/X86/tail-merge.ll
> Modified:
>     llvm/trunk/lib/CodeGen/BranchFolding.cpp
>     llvm/trunk/test/DebugInfo/COFF/local-variables.ll
>
> Modified: llvm/trunk/lib/CodeGen/BranchFolding.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/BranchFolding.cpp?rev=285212&r1=285211&r2=285212&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/BranchFolding.cpp (original)
> +++ llvm/trunk/lib/CodeGen/BranchFolding.cpp Wed Oct 26 12:01:47 2016
> @@ -720,8 +720,6 @@ bool BranchFolder::CreateCommonTailOnlyB
>      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);
>
> @@ -898,6 +896,11 @@ bool BranchFolder::TryTailMergeBlocks(Ma
>      // 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()
>
> Modified: llvm/trunk/test/DebugInfo/COFF/local-variables.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/COFF/local-variables.ll?rev=285212&r1=285211&r2=285212&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/COFF/local-variables.ll (original)
> +++ llvm/trunk/test/DebugInfo/COFF/local-variables.ll Wed Oct 26 12:01:47
> 2016
> @@ -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
> @@ -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 [
> @@ -159,7 +157,7 @@
>  ; OBJ:      LocalVariableAddrRange {
>  ; OBJ:        OffsetStart: .text+0x2D
>  ; OBJ:        ISectStart: 0x0
> -; OBJ:        Range: 0x24
> +; OBJ:        Range: 0x1F
>  ; OBJ:      }
>  ; OBJ:    }
>  ; OBJ:    InlineSite {
> @@ -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 {
>
> Added: llvm/trunk/test/DebugInfo/X86/tail-merge.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/X86/tail-merge.ll?rev=285212&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/DebugInfo/X86/tail-merge.ll (added)
> +++ llvm/trunk/test/DebugInfo/X86/tail-merge.ll Wed Oct 26 12:01:47 2016
> @@ -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)
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161031/41854a59/attachment.html>


More information about the llvm-commits mailing list