[llvm] r248168 - Factor replacement of stores of FP constants into new function

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 21 11:21:59 PDT 2015


On Mon, Sep 21, 2015 at 9:01 AM Matt Arsenault via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: arsenm
> Date: Mon Sep 21 10:59:43 2015
> New Revision: 248168
>
> URL: http://llvm.org/viewvc/llvm-project?rev=248168&view=rev
> Log:
> Factor replacement of stores of FP constants into new function
>
> Modified:
>     llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=248168&r1=248167&r2=248168&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon Sep 21
> 10:59:43 2015
> @@ -301,6 +301,7 @@ namespace {
>      SDValue visitLOAD(SDNode *N);
>
>      SDValue replaceStoreChain(StoreSDNode *ST, SDValue BetterChain);
> +    SDValue replaceStoreOfFPConstant(StoreSDNode *ST);
>
>      SDValue visitSTORE(SDNode *N);
>      SDValue visitINSERT_VECTOR_ELT(SDNode *N);
> @@ -11361,6 +11362,102 @@ SDValue DAGCombiner::replaceStoreChain(S
>    return CombineTo(ST, Token, false);
>  }
>
> +SDValue DAGCombiner::replaceStoreOfFPConstant(StoreSDNode *ST) {
> +  SDValue Value = ST->getValue();
> +  if (Value.getOpcode() == ISD::TargetConstantFP)
> +    return SDValue();
> +
> +  SDLoc DL(ST);
> +
> +  SDValue Chain = ST->getChain();
> +  SDValue Ptr = ST->getBasePtr();
> +
> +  const ConstantFPSDNode *CFP = cast<ConstantFPSDNode>(Value);
> +
> +  // NOTE: If the original store is volatile, this transform must not
> increase
> +  // the number of stores.  For example, on x86-32 an f64 can be stored
> in one
> +  // processor operation but an i64 (which is not legal) requires two.
> So the
> +  // transform should not be done in this case.
> +
> +  SDValue Tmp;
> +  switch (CFP->getSimpleValueType(0).SimpleTy) {
> +  default:
> +    llvm_unreachable("Unknown FP type");
> +  case MVT::f16:    // We don't do this for these yet.
> +  case MVT::f80:
> +  case MVT::f128:
> +  case MVT::ppcf128:
> +    return SDValue();
> +  case MVT::f32:
> +    if ((isTypeLegal(MVT::i32) && !LegalOperations && !ST->isVolatile())
> ||
> +        TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i32)) {
> +      ;
>

Why is this still here?


> +      Tmp = DAG.getConstant((uint32_t)CFP->getValueAPF().
> +                            bitcastToAPInt().getZExtValue(), SDLoc(CFP),
> +                            MVT::i32);
> +      SDValue NewSt = DAG.getStore(Chain, DL, Tmp,
> +                                   Ptr, ST->getMemOperand());
> +
> +      dbgs() << "Replacing FP constant: ";
>

No no no... This is spewing all over my builds now...


> +      Value->dump(&DAG);
> +
> +      if (cast<StoreSDNode>(NewSt)->getMemoryVT() != MVT::i32) {
> +        dbgs() << "Different memoryvt\n";
>

Along with all of this.

Was this really ready to be committed?



> +      } else {
> +        dbgs() << "same memoryvt\n";
> +      }
> +
> +
> +      return NewSt;
> +    }
> +
> +    return SDValue();
> +  case MVT::f64:
> +    if ((TLI.isTypeLegal(MVT::i64) && !LegalOperations &&
> +         !ST->isVolatile()) ||
> +        TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i64)) {
> +      ;
> +      Tmp = DAG.getConstant(CFP->getValueAPF().bitcastToAPInt().
> +                            getZExtValue(), SDLoc(CFP), MVT::i64);
> +      return DAG.getStore(Chain, DL, Tmp,
> +                          Ptr, ST->getMemOperand());
> +    }
> +
> +    if (!ST->isVolatile() &&
> +        TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i32)) {
> +      // Many FP stores are not made apparent until after legalize, e.g.
> for
> +      // argument passing.  Since this is so common, custom legalize the
> +      // 64-bit integer store into two 32-bit stores.
> +      uint64_t Val = CFP->getValueAPF().bitcastToAPInt().getZExtValue();
> +      SDValue Lo = DAG.getConstant(Val & 0xFFFFFFFF, SDLoc(CFP),
> MVT::i32);
> +      SDValue Hi = DAG.getConstant(Val >> 32, SDLoc(CFP), MVT::i32);
> +      if (DAG.getDataLayout().isBigEndian())
> +        std::swap(Lo, Hi);
> +
> +      unsigned Alignment = ST->getAlignment();
> +      bool isVolatile = ST->isVolatile();
> +      bool isNonTemporal = ST->isNonTemporal();
> +      AAMDNodes AAInfo = ST->getAAInfo();
> +
> +      SDValue St0 = DAG.getStore(Chain, DL, Lo,
> +                                 Ptr, ST->getPointerInfo(),
> +                                 isVolatile, isNonTemporal,
> +                                 ST->getAlignment(), AAInfo);
> +      Ptr = DAG.getNode(ISD::ADD, DL, Ptr.getValueType(), Ptr,
> +                        DAG.getConstant(4, DL, Ptr.getValueType()));
> +      Alignment = MinAlign(Alignment, 4U);
> +      SDValue St1 = DAG.getStore(Chain, DL, Hi,
> +                                 Ptr,
> ST->getPointerInfo().getWithOffset(4),
> +                                 isVolatile, isNonTemporal,
> +                                 Alignment, AAInfo);
> +      return DAG.getNode(ISD::TokenFactor, DL, MVT::Other,
> +                         St0, St1);
> +    }
> +
> +    return SDValue();
> +  }
> +}
> +
>  SDValue DAGCombiner::visitSTORE(SDNode *N) {
>    StoreSDNode *ST  = cast<StoreSDNode>(N);
>    SDValue Chain = ST->getChain();
> @@ -11389,78 +11486,13 @@ SDValue DAGCombiner::visitSTORE(SDNode *
>      return Chain;
>
>    // Turn 'store float 1.0, Ptr' -> 'store int 0x12345678, Ptr'
> -  if (ConstantFPSDNode *CFP = dyn_cast<ConstantFPSDNode>(Value)) {
> -    // NOTE: If the original store is volatile, this transform must not
> increase
> -    // the number of stores.  For example, on x86-32 an f64 can be stored
> in one
> -    // processor operation but an i64 (which is not legal) requires two.
> So the
> -    // transform should not be done in this case.
> -    if (Value.getOpcode() != ISD::TargetConstantFP) {
> -      SDValue Tmp;
> -      switch (CFP->getSimpleValueType(0).SimpleTy) {
> -      default: llvm_unreachable("Unknown FP type");
> -      case MVT::f16:    // We don't do this for these yet.
> -      case MVT::f80:
> -      case MVT::f128:
> -      case MVT::ppcf128:
> -        break;
> -      case MVT::f32:
> -        if ((isTypeLegal(MVT::i32) && !LegalOperations &&
> !ST->isVolatile()) ||
> -            TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i32)) {
> -          ;
> -          Tmp = DAG.getConstant((uint32_t)CFP->getValueAPF().
> -                              bitcastToAPInt().getZExtValue(), SDLoc(CFP),
> -                              MVT::i32);
> -          return DAG.getStore(Chain, SDLoc(N), Tmp,
> -                              Ptr, ST->getMemOperand());
> -        }
> -        break;
> -      case MVT::f64:
> -        if ((TLI.isTypeLegal(MVT::i64) && !LegalOperations &&
> -             !ST->isVolatile()) ||
> -            TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i64)) {
> -          ;
> -          Tmp = DAG.getConstant(CFP->getValueAPF().bitcastToAPInt().
> -                                getZExtValue(), SDLoc(CFP), MVT::i64);
> -          return DAG.getStore(Chain, SDLoc(N), Tmp,
> -                              Ptr, ST->getMemOperand());
> -        }
> -
> -        if (!ST->isVolatile() &&
> -            TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i32)) {
> -          // Many FP stores are not made apparent until after legalize,
> e.g. for
> -          // argument passing.  Since this is so common, custom legalize
> the
> -          // 64-bit integer store into two 32-bit stores.
> -          uint64_t Val =
> CFP->getValueAPF().bitcastToAPInt().getZExtValue();
> -          SDValue Lo = DAG.getConstant(Val & 0xFFFFFFFF, SDLoc(CFP),
> MVT::i32);
> -          SDValue Hi = DAG.getConstant(Val >> 32, SDLoc(CFP), MVT::i32);
> -          if (DAG.getDataLayout().isBigEndian())
> -            std::swap(Lo, Hi);
> -
> -          unsigned Alignment = ST->getAlignment();
> -          bool isVolatile = ST->isVolatile();
> -          bool isNonTemporal = ST->isNonTemporal();
> -          AAMDNodes AAInfo = ST->getAAInfo();
> -
> -          SDLoc DL(N);
> -
> -          SDValue St0 = DAG.getStore(Chain, SDLoc(ST), Lo,
> -                                     Ptr, ST->getPointerInfo(),
> -                                     isVolatile, isNonTemporal,
> -                                     ST->getAlignment(), AAInfo);
> -          Ptr = DAG.getNode(ISD::ADD, DL, Ptr.getValueType(), Ptr,
> -                            DAG.getConstant(4, DL, Ptr.getValueType()));
> -          Alignment = MinAlign(Alignment, 4U);
> -          SDValue St1 = DAG.getStore(Chain, SDLoc(ST), Hi,
> -                                     Ptr,
> ST->getPointerInfo().getWithOffset(4),
> -                                     isVolatile, isNonTemporal,
> -                                     Alignment, AAInfo);
> -          return DAG.getNode(ISD::TokenFactor, DL, MVT::Other,
> -                             St0, St1);
> -        }
> -
> -        break;
> -      }
> -    }
> +  //
> +  // Make sure to do this only after attempting to merge stores in order
> to
> +  //  avoid changing the types of some subset of stores due to visit
> order,
> +  //  preventing their merging.
> +  if (isa<ConstantFPSDNode>(Value)) {
> +    if (SDValue NewSt = replaceStoreOfFPConstant(ST))
> +      return NewSt;
>    }
>
>    // Try to infer better alignment information than the store already has.
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150921/e661ea69/attachment.html>


More information about the llvm-commits mailing list