[llvm] r203829 - Fix for http://llvm.org/bugs/show_bug.cgi?id=18590
Romanova, Katya
Katya_Romanova at playstation.sony.com
Thu Mar 13 13:47:21 PDT 2014
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