[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