[llvm] r181423 - PPCInstrInfo::optimizeCompareInstr should not optimize FP compares

[Bill Wendling]

Hi Hal,

I'm okay with the code owners merging the changes into the tree. Just let me know when you do it. :-)

Thanks!

-bw

On May 8, 2013, at 5:23 AM, Hal Finkel <hfinkel at anl.gov> wrote:

> Bill,
> 
> This should go into the release branch. Do you prefer to do all of the merging yourself, or would you prefer that the code owners do it?
> 
> Thanks again,
> Hal
> 
> ----- Original Message -----
>> Author: hfinkel
>> Date: Wed May  8 07:16:14 2013
>> New Revision: 181423
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=181423&view=rev
>> Log:
>> PPCInstrInfo::optimizeCompareInstr should not optimize FP compares
>> 
>> The floating-point record forms on PPC don't set the condition
>> register bits
>> based on a comparison with zero (like the integer record forms do),
>> but rather
>> based on the exception status bits.
>> 
>> Modified:
>>    llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp
>>    llvm/trunk/test/CodeGen/PowerPC/optcmp.ll
>> 
>> Modified: llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp?rev=181423&r1=181422&r2=181423&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp (original)
>> +++ llvm/trunk/lib/Target/PowerPC/PPCInstrInfo.cpp Wed May  8
>> 07:16:14 2013
>> @@ -1096,8 +1096,11 @@ bool PPCInstrInfo::optimizeCompareInstr(
>> 
>>   int OpC = CmpInstr->getOpcode();
>>   unsigned CRReg = CmpInstr->getOperand(0).getReg();
>> -  bool isFP = OpC == PPC::FCMPUS || OpC == PPC::FCMPUD;
>> -  unsigned CRRecReg = isFP ? PPC::CR1 : PPC::CR0;
>> +
>> +  // FP record forms set CR1 based on the execption status bits, not
>> a
>> +  // comparison with zero.
>> +  if (OpC == PPC::FCMPUS || OpC == PPC::FCMPUD)
>> +    return false;
>> 
>>   // The record forms set the condition register based on a signed
>>   comparison
>>   // with zero (so says the ISA manual). This is not as
>>   straightforward as it
>> @@ -1140,9 +1143,9 @@ bool PPCInstrInfo::optimizeCompareInstr(
>>         equalityOnly = true;
>>       } else
>>         return false;
>> -    } else if (!isFP)
>> +    } else
>>       equalityOnly = is64BitUnsignedCompare;
>> -  } else if (!isFP)
>> +  } else
>>     equalityOnly = is32BitUnsignedCompare;
>> 
>>   if (equalityOnly) {
>> @@ -1211,8 +1214,8 @@ bool PPCInstrInfo::optimizeCompareInstr(
>>     unsigned IOpC = Instr.getOpcode();
>> 
>>     if (&*I != CmpInstr && (
>> -        Instr.modifiesRegister(CRRecReg, TRI) ||
>> -        Instr.readsRegister(CRRecReg, TRI)))
>> +        Instr.modifiesRegister(PPC::CR0, TRI) ||
>> +        Instr.readsRegister(PPC::CR0, TRI)))
>>       // This instruction modifies or uses the record condition
>>       register after
>>       // the one we want to change. While we could do this
>>       transformation, it
>>       // would likely not be profitable. This transformation removes
>>       one
>> @@ -1232,15 +1235,6 @@ bool PPCInstrInfo::optimizeCompareInstr(
>>       break;
>>     }
>> 
>> -    if (isFP && (IOpC == PPC::FSUB || IOpC == PPC::FSUBS) &&
>> -        ((Instr.getOperand(1).getReg() == SrcReg &&
>> -          Instr.getOperand(2).getReg() == SrcReg2) ||
>> -        (Instr.getOperand(1).getReg() == SrcReg2 &&
>> -         Instr.getOperand(2).getReg() == SrcReg))) {
>> -      Sub = &*I;
>> -      break;
>> -    }
>> -
>>     if (I == B)
>>       // The 'and' is below the comparison instruction.
>>       return false;
>> @@ -1286,8 +1280,7 @@ bool PPCInstrInfo::optimizeCompareInstr(
>> 
>>     // The operands to subf are the opposite of sub, so only in the
>>     fixed-point
>>     // case, invert the order.
>> -    if (!isFP)
>> -      ShouldSwap = !ShouldSwap;
>> +    ShouldSwap = !ShouldSwap;
>>   }
>> 
>>   if (ShouldSwap)
>> @@ -1326,7 +1319,7 @@ bool PPCInstrInfo::optimizeCompareInstr(
>>   MachineBasicBlock::iterator MII = MI;
>>   BuildMI(*MI->getParent(), llvm::next(MII), MI->getDebugLoc(),
>>           get(TargetOpcode::COPY), CRReg)
>> -    .addReg(CRRecReg, MIOpC != NewOpC ? RegState::Kill : 0);
>> +    .addReg(PPC::CR0, MIOpC != NewOpC ? RegState::Kill : 0);
>> 
>>   if (MIOpC != NewOpC) {
>>     // We need to be careful here: we're replacing one instruction
>>     with
>> 
>> Modified: llvm/trunk/test/CodeGen/PowerPC/optcmp.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/PowerPC/optcmp.ll?rev=181423&r1=181422&r2=181423&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/CodeGen/PowerPC/optcmp.ll (original)
>> +++ llvm/trunk/test/CodeGen/PowerPC/optcmp.ll Wed May  8 07:16:14
>> 2013
>> @@ -118,7 +118,7 @@ entry:
>>   ret double %cond
>> 
>> ; CHECK: @food
>> -; CHECK: fsub. 0, 1, 2
>> +; CHECK-NOT: fsub. 0, 1, 2
>> ; CHECK: stfd 0, 0(5)
>> }
>> 
>> @@ -131,7 +131,7 @@ entry:
>>   ret float %cond
>> 
>> ; CHECK: @foof
>> -; CHECK: fsubs. 0, 1, 2
>> +; CHECK-NOT: fsubs. 0, 1, 2
>> ; CHECK: stfs 0, 0(5)
>> }
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>