[llvm] r285212 - Reapply: "Remove debug location from common tail when tail-merging"
Robert Lougher via llvm-commits
llvm-commits at lists.llvm.org
Wed Nov 2 05:34:24 PDT 2016
Hi David,
I reverted the commit because a ubsan test was failing. It turned out the
test was fragile, so I fixed the test in r285208 (commit message below).
Thanks,
Rob.
[ubsan] Fix vptr.cpp test to be more resilient. NFC.
The test contains a switch statement in which two of the cases are
tail-merged, with the call to __ubsan_handle_dynamic_type_cache_miss_abort
in the common tail. When tail-merging occurs, the debug location of the
tail is randomly taken from one of the merge inputs. Luckily for the test,
the expected line number in the check is the one which is chosen by the
tail-merge. However, if the switch cases are re-ordered the test will
fail.
This patch disables tail-merge, making the test resilient to changes
in tail-merge, and unblocking review D25742. It does not change the
semantics of the test.
On 31 October 2016 at 17:14, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> 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/20161102/6beeba9c/attachment.html>
More information about the llvm-commits
mailing list