[llvm] r203829 - Fix for http://llvm.org/bugs/show_bug.cgi?id=18590
Kevin Enderby
enderby at apple.com
Thu Mar 13 13:51:07 PDT 2014
Hi Katya,
I made a guess as to the needed triple being -mtriple=x86_64-linux and added to the test case in r203844. Feel free to correct that if I’m wrong. Without it I suspect it uses the native compiler which may not be what you are working with.
Kev
On Mar 13, 2014, at 1:47 PM, Romanova, Katya <Katya_Romanova at playstation.sony.com> wrote:
> Hi Kevin,
>
> Do you know for which triple the test failed? I will check this triple and I correct the testcase accordingly (either by defining a specific triple or allowing "_").
>
> After I make a correction, do I need to get a review or approval? If so, who will review or approve?
> Was my change committed and reverted later, or not committed at all?
>
> Katya.
>
>
> -----Original Message-----
> From: Kevin Enderby [mailto:enderby at apple.com]
> Sent: Thursday, March 13, 2014 1:06 PM
> To: Romanova, Katya
> Cc: Kevin Enderby; llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm] r203829 - Fix for http://llvm.org/bugs/show_bug.cgi?id=18590
>
> Hi Ekaterina,
>
> Looks like you may need to adjust your test case, maybe an needed explicit triple? or some added under bars? We are seeing a failure on our internal bots over here:
>
> Begin forwarded message:
>
>> LOGS:
>> Last 15 lines of 'stdio':
>> PASS: LLVM-Unit :: Transforms/Utils/Release/UtilsTests/SpecialCaseListTest.Substring (17415 of 17416)
>> PASS: LLVM-Unit ::
>> Support/Release/SupportTests/ProgramTest.TestExecuteNoWait (17416 of
>> 17416)
>>
>> 1 warning(s) in tests.
>> Testing Time: 51.74s
>> ********************
>> Failing Tests (1):
>> LLVM :: CodeGen/X86/dbg-changes-codegen.ll
>>
>> Expected Passes : 14982
>> Expected Failures : 67
>> Unsupported Tests : 2366
>> Unexpected Failures: 1
>> make[1]: *** [check-local-all] Error 1
>> make: *** [check-all] Error 2
>>
>> Last 15 lines of 'LLVM::dbg-changes-codegen.ll':
>> cmpq %rdi, _pfoo(%rip)
>> ^
>> .../Sources/clang/src/test/CodeGen/X86/dbg-changes-codegen.ll:10:10: error: expected string not found in input
>> ; CHECK: cmpq {{%[a-z]+}}, wibble2(%rip)
>> ^
>> <stdin>:13:2: note: scanning from here
>> .align 4, 0x90
>> ^
>> <stdin>:18:2: note: possible intended match here
>> cmpq %rax, _wibble2(%rip)
>> ^
>>
>> --
>>
>> ********************
>>
>> Last 15 lines of 'warnings (2)':
>> lit.py: discovery.py:190: warning: test suite 'Extra Tools Unit Tests' contained no tests
>> warning: ignoring debug info with an invalid version (0) in <stdin>
>
>
> Thanks for your help, the current build czar, Kev
>
> On Mar 13, 2014, at 11:47 AM, Ekaterina Romanova <katya_romanova at playstation.sony.com> wrote:
>
>> Author: kromanova
>> Date: Thu Mar 13 13:47:12 2014
>> New Revision: 203829
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=203829&view=rev
>> Log:
>> Fix for http://llvm.org/bugs/show_bug.cgi?id=18590
>>
>> This patch fixes the bug in peephole optimization that folds a load which defines one vreg into the one and only use of that vreg. With debug info, a DBG_VALUE that referenced the vreg considered to be a use, preventing the optimization. The fix is to ignore DBG_VALUE's during the optimization, and undef a DBG_VALUE that references a vreg that gets removed.
>> Patch by Trevor Smigiel!
>>
>>
>> Added:
>> llvm/trunk/test/CodeGen/X86/dbg-changes-codegen.ll
>> Modified:
>> llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h
>> llvm/trunk/lib/CodeGen/DeadMachineInstructionElim.cpp
>> llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp
>> llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp
>>
>> Modified: llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/Ma
>> chineRegisterInfo.h?rev=203829&r1=203828&r2=203829&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h (original)
>> +++ llvm/trunk/include/llvm/CodeGen/MachineRegisterInfo.h Thu Mar 13
>> +++ 13:47:12 2014
>> @@ -510,6 +510,10 @@ public:
>> return Hint.first ? 0 : Hint.second;
>> }
>>
>> + /// markUsesInDebugValueAsUndef - Mark every DBG_VALUE referencing
>> + the /// specified register as undefined which causes the DBG_VALUE
>> + to be /// deleted during LiveDebugVariables analysis.
>> + void markUsesInDebugValueAsUndef(unsigned Reg) const;
>>
>> //===--------------------------------------------------------------------===//
>> // Physical Register Use Info
>>
>> Modified: llvm/trunk/lib/CodeGen/DeadMachineInstructionElim.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/DeadMachine
>> InstructionElim.cpp?rev=203829&r1=203828&r2=203829&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/lib/CodeGen/DeadMachineInstructionElim.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/DeadMachineInstructionElim.cpp Thu Mar 13
>> +++ 13:47:12 2014
>> @@ -127,17 +127,7 @@ bool DeadMachineInstructionElim::runOnMa
>> unsigned Reg = MO.getReg();
>> if (!TargetRegisterInfo::isVirtualRegister(Reg))
>> continue;
>> - MachineRegisterInfo::use_iterator nextI;
>> - for (MachineRegisterInfo::use_iterator I = MRI->use_begin(Reg),
>> - E = MRI->use_end(); I!=E; I=nextI) {
>> - nextI = std::next(I); // I is invalidated by the setReg
>> - MachineOperand& Use = I.getOperand();
>> - MachineInstr *UseMI = Use.getParent();
>> - if (UseMI==MI)
>> - continue;
>> - assert(Use.isDebug());
>> - UseMI->getOperand(0).setReg(0U);
>> - }
>> + MRI->markUsesInDebugValueAsUndef(Reg);
>> }
>> AnyChanges = true;
>> MI->eraseFromParent();
>>
>> Modified: llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineRegi
>> sterInfo.cpp?rev=203829&r1=203828&r2=203829&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/MachineRegisterInfo.cpp Thu Mar 13 13:47:12
>> +++ 2014
>> @@ -414,3 +414,18 @@ bool MachineRegisterInfo::isConstantPhys
>> return false;
>> return true;
>> }
>> +
>> +/// markUsesInDebugValueAsUndef - Mark every DBG_VALUE referencing
>> +the /// specified register as undefined which causes the DBG_VALUE to
>> +be /// deleted during LiveDebugVariables analysis.
>> +void MachineRegisterInfo::markUsesInDebugValueAsUndef(unsigned Reg)
>> +const {
>> + // Mark any DBG_VALUE that uses Reg as undef (but don't delete it.)
>> + MachineRegisterInfo::use_iterator nextI;
>> + for (use_iterator I = use_begin(Reg), E = use_end(); I != E; I = nextI) {
>> + nextI = std::next(I); // I is invalidated by the setReg
>> + MachineOperand& Use = I.getOperand();
>> + MachineInstr *UseMI = Use.getParent();
>> + if (UseMI->isDebugValue())
>> + UseMI->getOperand(0).setReg(0U);
>> + }
>> +}
>>
>> Modified: llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/PeepholeOpt
>> imizer.cpp?rev=203829&r1=203828&r2=203829&view=diff
>> ======================================================================
>> ========
>> --- llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/PeepholeOptimizer.cpp Thu Mar 13 13:47:12
>> +++ 2014
>> @@ -505,12 +505,12 @@ bool PeepholeOptimizer::isLoadFoldable(M
>> return false;
>>
>> unsigned Reg = MI->getOperand(0).getReg();
>> - // To reduce compilation time, we check MRI->hasOneUse when
>> inserting
>> + // To reduce compilation time, we check MRI->hasOneNonDBGUse when
>> + inserting
>> // loads. It should be checked when processing uses of the load, since
>> // uses can be removed during peephole.
>> if (!MI->getOperand(0).getSubReg() &&
>> TargetRegisterInfo::isVirtualRegister(Reg) &&
>> - MRI->hasOneUse(Reg)) {
>> + MRI->hasOneNonDBGUse(Reg)) {
>> FoldAsLoadDefReg = Reg;
>> return true;
>> }
>> @@ -594,10 +594,14 @@ bool PeepholeOptimizer::runOnMachineFunc
>> ++MII;
>> LocalMIs.insert(MI);
>>
>> + // Skip debug values. They should not affect this peephole optimization.
>> + if (MI->isDebugValue())
>> + continue;
>> +
>> // If there exists an instruction which belongs to the following
>> // categories, we will discard the load candidate.
>> if (MI->isPosition() || MI->isPHI() || MI->isImplicitDef() ||
>> - MI->isKill() || MI->isInlineAsm() || MI->isDebugValue() ||
>> + MI->isKill() || MI->isInlineAsm() ||
>> MI->hasUnmodeledSideEffects()) {
>> FoldAsLoadDefReg = 0;
>> continue;
>> @@ -633,6 +637,9 @@ bool PeepholeOptimizer::runOnMachineFunc
>> if (!isLoadFoldable(MI, FoldAsLoadDefReg) && FoldAsLoadDefReg) {
>> // We need to fold load after optimizeCmpInstr, since optimizeCmpInstr
>> // can enable folding by converting SUB to CMP.
>> + // Save FoldAsLoadDefReg because optimizeLoadInstr() resets it and we
>> + // need it for markUsesInDebugValueAsUndef().
>> + unsigned FoldedReg = FoldAsLoadDefReg;
>> MachineInstr *DefMI = 0;
>> MachineInstr *FoldMI = TII->optimizeLoadInstr(MI, MRI,
>>
>> FoldAsLoadDefReg, DefMI); @@ -645,6 +652,7 @@ bool PeepholeOptimizer::runOnMachineFunc
>> LocalMIs.insert(FoldMI);
>> MI->eraseFromParent();
>> DefMI->eraseFromParent();
>> + MRI->markUsesInDebugValueAsUndef(FoldedReg);
>> ++NumLoadFold;
>>
>> // MI is replaced with FoldMI.
>>
>> Added: llvm/trunk/test/CodeGen/X86/dbg-changes-codegen.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/dbg-ch
>> anges-codegen.ll?rev=203829&view=auto
>> ======================================================================
>> ========
>> --- llvm/trunk/test/CodeGen/X86/dbg-changes-codegen.ll (added)
>> +++ llvm/trunk/test/CodeGen/X86/dbg-changes-codegen.ll Thu Mar 13
>> +++ 13:47:12 2014
>> @@ -0,0 +1,83 @@
>> +; RUN: llc < %s -march=x86-64 | FileCheck %s
>> +
>> +; The Peephole optimizer should fold the load into the cmp even with debug info.
>> +; CHECK-LABEL: _ZN3Foo3batEv
>> +; CHECK-NOT: movq pfoo
>> +; CHECK: cmpq {{%[a-z]+}}, pfoo(%rip) ; ; CHECK-LABEL: _Z3bazv ;
>> +CHECK-NOT: movq wibble2 ; CHECK: cmpq {{%[a-z]+}}, wibble2(%rip)
>> +
>> +; Regenerate test with this command:
>> +; clang -emit-llvm -S -O2 -g
>> +; from this source:
>> +; struct Foo {
>> +; bool bat();
>> +; bool operator==(Foo &arg) { return (this == &arg); }
>> +; };
>> +; Foo *pfoo;
>> +; bool Foo::bat() { return (*this == *pfoo); }
>> +;
>> +; struct Wibble {
>> +; int x;
>> +; } *wibble1, *wibble2;
>> +; struct Flibble {
>> +; void bar(Wibble *c) {
>> +; if (c < wibble2)
>> +; wibble2 = 0;
>> +; c->x = 0;
>> +; }
>> +; } flibble;
>> +; void baz() { flibble.bar(wibble1); }
>> +
>> +%struct.Foo = type { i8 }
>> +%struct.Wibble = type { i32 }
>> +%struct.Flibble = type { i8 }
>> +
>> + at pfoo = global %struct.Foo* null, align 8
>> + at wibble1 = global %struct.Wibble* null, align 8
>> + at wibble2 = global %struct.Wibble* null, align 8 @flibble = global
>> +%struct.Flibble zeroinitializer, align 1
>> +
>> +; Function Attrs: nounwind readonly uwtable define zeroext i1
>> + at _ZN3Foo3batEv(%struct.Foo* %this) #0 align 2 {
>> +entry:
>> + %0 = load %struct.Foo** @pfoo, align 8
>> + tail call void @llvm.dbg.value(metadata !{%struct.Foo* %0}, i64 0,
>> +metadata !62)
>> + %cmp.i = icmp eq %struct.Foo* %0, %this
>> + ret i1 %cmp.i
>> +}
>> +
>> +; Function Attrs: nounwind uwtable
>> +define void @_Z3bazv() #1 {
>> +entry:
>> + %0 = load %struct.Wibble** @wibble1, align 8
>> + tail call void @llvm.dbg.value(metadata !64, i64 0, metadata !65)
>> + %1 = load %struct.Wibble** @wibble2, align 8
>> + %cmp.i = icmp ugt %struct.Wibble* %1, %0
>> + br i1 %cmp.i, label %if.then.i, label
>> +%_ZN7Flibble3barEP6Wibble.exit
>> +
>> +if.then.i: ; preds = %entry
>> + store %struct.Wibble* null, %struct.Wibble** @wibble2, align 8
>> + br label %_ZN7Flibble3barEP6Wibble.exit
>> +
>> +_ZN7Flibble3barEP6Wibble.exit: ; preds = %entry, %if.then.i
>> + %x.i = getelementptr inbounds %struct.Wibble* %0, i64 0, i32 0
>> + store i32 0, i32* %x.i, align 4
>> + ret void
>> +}
>> +
>> +; Function Attrs: nounwind readnone
>> +declare void @llvm.dbg.value(metadata, i64, metadata) #2
>> +
>> +attributes #0 = { nounwind readonly uwtable
>> +"less-precise-fpmad"="false" "no-frame-pointer-elim"="false"
>> +"no-frame-pointer-elim-non-leaf"="false" "no-infs-fp-math"="false"
>> +"no-nans-fp-math"="false" "unsafe-fp-math"="false"
>> +"use-soft-float"="false" } attributes #1 = { nounwind uwtable
>> +"less-precise-fpmad"="false" "no-frame-pointer-elim"="false"
>> +"no-frame-pointer-elim-non-leaf"="false" "no-infs-fp-math"="false"
>> +"no-nans-fp-math"="false" "unsafe-fp-math"="false"
>> +"use-soft-float"="false" } attributes #2 = { nounwind readnone }
>> +
>> +
>> +!17 = metadata !{i32 786448, null, null, null, i32 0, i64 0, i64 0,
>> +i64 0, i32 0, null} ; [ DW_TAG_reference_type ] [line 0, size 0,
>> +align 0, offset 0] [from Foo]
>> +!45 = metadata !{i32 786447, null, null, metadata !"", i32 0, i64 64,
>> +i64 64, i64 0, i32 0, null} ; [ DW_TAG_pointer_type ] [line 0, size
>> +64, align 64, offset 0] [from Flibble]
>> +!62 = metadata !{i32 786689, null, metadata !"arg", null, i32
>> +33554436, metadata !17, i32 0, null} ; [ DW_TAG_arg_variable ] [arg]
>> +[line 4]
>> +!64 = metadata !{%struct.Flibble* undef}
>> +!65 = metadata !{i32 786689, null, metadata !"this", null, i32
>> +16777229, metadata !45, i32 1088, null} ; [ DW_TAG_arg_variable ]
>> +[this] [line 13]
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
More information about the llvm-commits
mailing list