[llvm] r220090 - [PowerPC] Change liveness testing in VSX FMA mutation pass
Hal Finkel
hfinkel at anl.gov
Fri Oct 17 14:18:22 PDT 2014
----- Original Message -----
> From: "Bill Schmidt" <wschmidt at linux.vnet.ibm.com>
> To: llvm-commits at cs.uiuc.edu
> Sent: Friday, October 17, 2014 4:02:44 PM
> Subject: [llvm] r220090 - [PowerPC] Change liveness testing in VSX FMA mutation pass
>
> Author: wschmidt
> Date: Fri Oct 17 16:02:44 2014
> New Revision: 220090
>
> URL: http://llvm.org/viewvc/llvm-project?rev=220090&view=rev
> Log:
> [PowerPC] Change liveness testing in VSX FMA mutation pass
>
> With VSX enabled, LLVM crashes when compiling
> test/CodeGen/PowerPC/fma.ll. I traced this to the liveness test
> that's revised in this patch. The interval test is designed to only
> work for virtual registers, but in this case the AddendSrcReg is
> physical. Since there is already a walk of the MIs between the
> AddendMI and the FMA, I added a check for def/kill of the
> AddendSrcReg
> in that loop. At Hal Finkel's request, I converted the liveness test
> to an assert restricted to virtual registers.
>
> I've changed the fma.ll test to have VSX and non-VSX variants so we
> can test both kinds of multiply-adds.
>
> Modified:
> llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp
> llvm/trunk/test/CodeGen/PowerPC/fma.ll
>
> Modified: llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp?rev=220090&r1=220089&r2=220090&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp (original)
> +++ llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp Fri Oct 17
> 16:02:44 2014
> @@ -1622,6 +1622,7 @@ protected:
> bool Changed = false;
>
> MachineRegisterInfo &MRI = MBB.getParent()->getRegInfo();
> + const TargetRegisterInfo *TRI = &TII->getRegisterInfo();
> for (MachineBasicBlock::iterator I = MBB.begin(), IE =
> MBB.end();
> I != IE; ++I) {
> MachineInstr *MI = I;
> @@ -1687,16 +1688,26 @@ protected:
> // In theory, there could be other uses of the addend copy
> before this
> // fma. We could deal with this, but that would require
> additional
> // logic below and I suspect it will not occur in any
> relevant
> - // situations.
> - bool OtherUsers = false;
> + // situations. Additionally, check whether the copy source
> is killed
> + // prior to the fma. In order to replace the addend here
> with the
> + // source of the copy, it must still be live here. We can't
> use
> + // interval testing for a physical register, so as long as
> we're
> + // walking the MIs we may as well test liveness here.
> + bool OtherUsers = false, KillsAddendSrc = false;
> for (auto J = std::prev(I), JE =
> MachineBasicBlock::iterator(AddendMI);
> - J != JE; --J)
> + J != JE; --J) {
> if
> (J->readsVirtualRegister(AddendMI->getOperand(0).getReg()))
> {
> OtherUsers = true;
> break;
> }
> + if (J->modifiesRegister(AddendSrcReg, TRI) ||
> + J->killsRegister(AddendSrcReg, TRI)) {
> + KillsAddendSrc = true;
> + break;
> + }
> + }
>
> - if (OtherUsers)
> + if (OtherUsers || KillsAddendSrc)
> continue;
>
> // Find one of the product operands that is killed by this
> instruction.
> @@ -1717,10 +1728,11 @@ protected:
> if (!KilledProdOp)
> continue;
>
> - // In order to replace the addend here with the source of
> the copy,
> - // it must still be live here.
> - if
> (!LIS->getInterval(AddendMI->getOperand(1).getReg()).liveAt(FMAIdx))
> - continue;
> + // For virtual registers, verify that the addend source
> register
> + // is live here (as should have been assured above).
> + if (TargetRegisterInfo::isVirtualRegister(AddendSrcReg))
> + assert(LIS->getInterval(AddendSrcReg).liveAt(FMAIdx) &&
> + "Addend source register is not available!");
Will this even compile in a non-asserts build? Regardless, please make this:
assert((!TargetRegisterInfo::isVirtualRegister(AddendSrcReg) || LIS->getInterval(AddendSrcReg).liveAt(FMAIdx)) && "Addend source register is not live!");
-Hal
>
> // Transform: (O2 * O3) + O1 -> (O2 * O1) + O3.
>
>
> Modified: llvm/trunk/test/CodeGen/PowerPC/fma.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/fma.ll?rev=220090&r1=220089&r2=220090&view=diff
> ==============================================================================
> --- llvm/trunk/test/CodeGen/PowerPC/fma.ll (original)
> +++ llvm/trunk/test/CodeGen/PowerPC/fma.ll Fri Oct 17 16:02:44 2014
> @@ -1,4 +1,5 @@
> -; RUN: llc < %s -march=ppc32 -fp-contract=fast | FileCheck %s
> +; RUN: llc < %s -march=ppc32 -fp-contract=fast -mattr=-vsx |
> FileCheck %s
> +; RUN: llc < %s -mtriple=powerpc64-unknown-linux-gnu
> -fp-contract=fast -mattr=+vsx | FileCheck -check-prefix=CHECK-VSX %s
>
> declare double @dummy1(double) #0
> declare double @dummy2(double, double) #0
> @@ -11,6 +12,10 @@ define double @test_FMADD1(double %A, do
> ; CHECK-LABEL: test_FMADD1:
> ; CHECK: fmadd
> ; CHECK-NEXT: blr
> +
> +; CHECK-VSX-LABEL: test_FMADD1:
> +; CHECK-VSX: xsmaddmdp
> +; CHECK-VSX-NEXT: blr
> }
>
> define double @test_FMADD2(double %A, double %B, double %C) {
> @@ -20,6 +25,10 @@ define double @test_FMADD2(double %A, do
> ; CHECK-LABEL: test_FMADD2:
> ; CHECK: fmadd
> ; CHECK-NEXT: blr
> +
> +; CHECK-VSX-LABEL: test_FMADD2:
> +; CHECK-VSX: xsmaddmdp
> +; CHECK-VSX-NEXT: blr
> }
>
> define double @test_FMSUB1(double %A, double %B, double %C) {
> @@ -29,6 +38,10 @@ define double @test_FMSUB1(double %A, do
> ; CHECK-LABEL: test_FMSUB1:
> ; CHECK: fmsub
> ; CHECK-NEXT: blr
> +
> +; CHECK-VSX-LABEL: test_FMSUB1:
> +; CHECK-VSX: xsmsubmdp
> +; CHECK-VSX-NEXT: blr
> }
>
> define double @test_FMSUB2(double %A, double %B, double %C, double
> %D) {
> @@ -40,6 +53,10 @@ define double @test_FMSUB2(double %A, do
> ; CHECK-LABEL: test_FMSUB2:
> ; CHECK: fmadd
> ; CHECK-NEXT: fmsub
> +
> +; CHECK-VSX-LABEL: test_FMSUB2:
> +; CHECK-VSX: xsmaddadp
> +; CHECK-VSX-NEXT: xsmsubmdp
> }
>
> define double @test_FNMADD1(double %A, double %B, double %C) {
> @@ -50,6 +67,10 @@ define double @test_FNMADD1(double %A, d
> ; CHECK-LABEL: test_FNMADD1:
> ; CHECK: fnmadd
> ; CHECK-NEXT: blr
> +
> +; CHECK-VSX-LABEL: test_FNMADD1:
> +; CHECK-VSX: xsnmaddmdp
> +; CHECK-VSX-NEXT: blr
> }
>
> define double @test_FNMADD2(double %A, double %B, double %C) {
> @@ -60,6 +81,10 @@ define double @test_FNMADD2(double %A, d
> ; CHECK-LABEL: test_FNMADD2:
> ; CHECK: fnmadd
> ; CHECK-NEXT: blr
> +
> +; CHECK-VSX-LABEL: test_FNMADD2:
> +; CHECK-VSX: xsnmaddmdp
> +; CHECK-VSX-NEXT: blr
> }
>
> define double @test_FNMSUB1(double %A, double %B, double %C) {
> @@ -69,6 +94,9 @@ define double @test_FNMSUB1(double %A, d
> ; CHECK-LABEL: test_FNMSUB1:
> ; CHECK: fnmsub
> ; CHECK-NEXT: blr
> +
> +; CHECK-VSX-LABEL: test_FNMSUB1:
> +; CHECK-VSX: xsnmsubmdp
> }
>
> define double @test_FNMSUB2(double %A, double %B, double %C) {
> @@ -79,6 +107,10 @@ define double @test_FNMSUB2(double %A, d
> ; CHECK-LABEL: test_FNMSUB2:
> ; CHECK: fnmsub
> ; CHECK-NEXT: blr
> +
> +; CHECK-VSX-LABEL: test_FNMSUB2:
> +; CHECK-VSX: xsnmsubmdp
> +; CHECK-VSX-NEXT: blr
> }
>
> define float @test_FNMSUBS(float %A, float %B, float %C) {
> @@ -89,4 +121,8 @@ define float @test_FNMSUBS(float %A, flo
> ; CHECK-LABEL: test_FNMSUBS:
> ; CHECK: fnmsubs
> ; CHECK-NEXT: blr
> +
> +; CHECK-VSX-LABEL: test_FNMSUBS:
> +; CHECK-VSX: fnmsubs
> +; CHECK-VSX-NEXT: blr
> }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
--
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory
More information about the llvm-commits
mailing list