[llvm-commits] [llvm] r41967 - in /llvm/trunk: include/llvm/ADT/APInt.h include/llvm/ADT/FoldingSet.h include/llvm/Constants.h lib/CodeGen/SelectionDAG/LegalizeDAG.cpp lib/CodeGen/SelectionDAG/SelectionDAG.cpp lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp lib/Support/APInt.cpp lib/Support/FoldingSet.cpp lib/Target/CBackend/CBackend.cpp lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86RegisterInfo.cpp lib/Transforms/Scalar/InstructionCombining.cpp lib/VMCore/Constants.cpp

[Chris Lattner]

> Remove the assumption that FP's are either float or
> double from some of the many places in the optimizers
> it appears, and do something reasonable with x86
> long double.

Nice!

> ====================================================================== 
> ========
> --- llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeDAG.cpp Fri Sep 14  
> 17:26:36 2007
> @@ -486,15 +486,23 @@
>    // double.
>    MVT::ValueType VT = CFP->getValueType(0);
>    bool isDouble = VT == MVT::f64;
> -  ConstantFP *LLVMC = ConstantFP::get(isDouble ? Type::DoubleTy :
> -                                      Type::FloatTy, CFP- 
> >getValueAPF());
> +  ConstantFP *LLVMC = ConstantFP::get(VT==MVT::f64 ? Type::DoubleTy :
> +                                      VT==MVT::f32 ? Type::FloatTy :
> +                                      VT==MVT::f80 ?  
> Type::X86_FP80Ty :
> +                                      VT==MVT::f128 ? Type::FP128Ty :
> +                                      VT==MVT::ppcf128 ?  
> Type::PPC_FP128Ty :
> +                                      Type::VoidTy,   // error
> +                                      CFP->getValueAPF());

This should probably use MVT::getTypeForValueType(VT)



> @@ -4609,12 +4620,16 @@
>      SDOperand FudgeInReg;
>      if (DestTy == MVT::f32)
>        FudgeInReg = DAG.getLoad(MVT::f32, DAG.getEntryNode(),  
> CPIdx, NULL, 0);
> -    else {
> -      assert(DestTy == MVT::f64 && "Unexpected conversion");
> +    else if (DestTy == MVT::f64)
>        // FIXME: Avoid the extend by construction the right  
> constantpool?
>        FudgeInReg = DAG.getExtLoad(ISD::EXTLOAD, MVT::f64,  
> DAG.getEntryNode(),
>                                    CPIdx, NULL, 0, MVT::f32);
> -    }
> +    else if (DestTy == MVT::f80)
> +      FudgeInReg = DAG.getExtLoad(ISD::EXTLOAD, MVT::f80,  
> DAG.getEntryNode(),
> +                                  CPIdx, NULL, 0, MVT::f32);
> +    else
> +      assert(0 && "Unexpected conversion");

Can this just be turned into something like:

      if (DestTy == MVT::f32)
        FudgeInReg = DAG.getLoad(MVT::f32, DAG.getEntryNode(), CPIdx,  
NULL, 0);
      else {
        assert(DestTy > MVT::f32 && "Unexpected conversion");
        // FIXME: Avoid the extend by construction the right  
constantpool?
        FudgeInReg = DAG.getExtLoad(ISD::EXTLOAD, DestTy,  
DAG.getEntryNode(),
                                    CPIdx, NULL, 0, MVT::f32);
      }

?


> @@ -4722,9 +4737,11 @@
>      if (DestVT == MVT::f64) {
>        // do nothing
>        Result = Sub;
> -    } else {
> +    } else if (DestVT == MVT::f32) {
>       // if f32 then cast to f32
>        Result = DAG.getNode(ISD::FP_ROUND, MVT::f32, Sub);
> +    } else if (DestVT == MVT::f80) {
> +      Result = DAG.getNode(ISD::FP_EXTEND, MVT::f80, Sub);
>      }

How about "if DestVT < f32 use round.  If DestVT > f64, use FP_EXTEND"?


> ====================================================================== 
> ========
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAG.cpp Fri Sep 14  
> 17:26:36 2007
> @@ -3724,9 +3714,15 @@
>    if (const ConstantSDNode *CSDN = dyn_cast<ConstantSDNode>(this)) {
>      cerr << "<" << CSDN->getValue() << ">";
>    } else if (const ConstantFPSDNode *CSDN =  
> dyn_cast<ConstantFPSDNode>(this)) {
> -    cerr << "<" << (&CSDN->getValueAPF().getSemantics() 
> ==&APFloat::IEEEsingle ?
> -                    CSDN->getValueAPF().convertToFloat() :
> -                    CSDN->getValueAPF().convertToDouble()) << ">";
> +    if (&CSDN->getValueAPF().getSemantics()==&APFloat::IEEEsingle)
> +      cerr << "<" << CSDN->getValueAPF().convertToFloat() << ">";
> +    else if (&CSDN->getValueAPF().getSemantics() 
> ==&APFloat::IEEEdouble)
> +      cerr << "<" << CSDN->getValueAPF().convertToDouble() << ">";
> +    else {
> +      cerr << "<APFloat(";
> +      CSDN->getValueAPF().convertToAPInt().dump();
> +      cerr << ")>";
> +    }

APFloat really needs a "convertToString" method. :)

> ====================================================================== 
> ========
> --- llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86RegisterInfo.cpp Fri Sep 14  
> 17:26:36 2007
> @@ -168,7 +168,11 @@
>      Opc = X86::MOV32_mr;
>    } else if (RC == &X86::GR16_RegClass) {
>      Opc = X86::MOV16_mr;
> +  } else if (RC == &X86::RFP80RegClass) {
> +    Opc = X86::ST_FpP80m;   // pops
>    } else if (RC == &X86::RFP64RegClass || RC == &X86::RSTRegClass) {
> +    /// FIXME spilling long double values as 64 bit does not work.
> +    /// We need RST80, unfortunately.

The FP Stack has 80 bit load and store instructions, what is the  
issue? or is it just a todo?

>

> +ConstantFP *ConstantFP::getNegativeZero(const Type *Ty) {
> +  APFloat apf = cast <ConstantFP>(Constant::getNullValue(Ty))- 
> >getValueAPF();
> +  apf.changeSign();
> +  return ConstantFP::get(Ty, apf);
> +}

This seems fairly expensive.  Why not just start by creating an  
APFloat of the appropriate zero type, changing its sign, then  
returning it.  Why start with a Constant?

Overall, very nice patch!

-Chris