[llvm] r220090 - [PowerPC] Change liveness testing in VSX FMA mutation pass
Renato Golin
renato.golin at linaro.org
Fri Oct 17 15:46:25 PDT 2014
Also, the test seem to be failing on our aarch64 bot:
http://lab.llvm.org:8011/builders/llvm-aarch64-linux/builds/4699/steps/test-llvm/logs/LLVM%3A%3Afma.ll
I don't know enough about PPC to infer what's wrong, and I don't have
access to the board to get you the full dump. Gabor, the owner of that
bot, is cc'd so if you need, maybe he can get you.
thanks,
-renato
On 17 October 2014 23:18, Hal Finkel <hfinkel at anl.gov> wrote:
> ----- 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
> _______________________________________________
> 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