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

Hal Finkel hfinkel at anl.gov
Wed May 8 05:23:36 PDT 2013


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
> 



More information about the llvm-commits mailing list