<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Sep 21, 2015 at 9:01 AM Matt Arsenault via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: arsenm<br>
Date: Mon Sep 21 10:59:43 2015<br>
New Revision: 248168<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=248168&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=248168&view=rev</a><br>
Log:<br>
Factor replacement of stores of FP constants into new function<br>
<br>
Modified:<br>
llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
<br>
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=248168&r1=248167&r2=248168&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=248168&r1=248167&r2=248168&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon Sep 21 10:59:43 2015<br>
@@ -301,6 +301,7 @@ namespace {<br>
SDValue visitLOAD(SDNode *N);<br>
<br>
SDValue replaceStoreChain(StoreSDNode *ST, SDValue BetterChain);<br>
+ SDValue replaceStoreOfFPConstant(StoreSDNode *ST);<br>
<br>
SDValue visitSTORE(SDNode *N);<br>
SDValue visitINSERT_VECTOR_ELT(SDNode *N);<br>
@@ -11361,6 +11362,102 @@ SDValue DAGCombiner::replaceStoreChain(S<br>
return CombineTo(ST, Token, false);<br>
}<br>
<br>
+SDValue DAGCombiner::replaceStoreOfFPConstant(StoreSDNode *ST) {<br>
+ SDValue Value = ST->getValue();<br>
+ if (Value.getOpcode() == ISD::TargetConstantFP)<br>
+ return SDValue();<br>
+<br>
+ SDLoc DL(ST);<br>
+<br>
+ SDValue Chain = ST->getChain();<br>
+ SDValue Ptr = ST->getBasePtr();<br>
+<br>
+ const ConstantFPSDNode *CFP = cast<ConstantFPSDNode>(Value);<br>
+<br>
+ // NOTE: If the original store is volatile, this transform must not increase<br>
+ // the number of stores. For example, on x86-32 an f64 can be stored in one<br>
+ // processor operation but an i64 (which is not legal) requires two. So the<br>
+ // transform should not be done in this case.<br>
+<br>
+ SDValue Tmp;<br>
+ switch (CFP->getSimpleValueType(0).SimpleTy) {<br>
+ default:<br>
+ llvm_unreachable("Unknown FP type");<br>
+ case MVT::f16: // We don't do this for these yet.<br>
+ case MVT::f80:<br>
+ case MVT::f128:<br>
+ case MVT::ppcf128:<br>
+ return SDValue();<br>
+ case MVT::f32:<br>
+ if ((isTypeLegal(MVT::i32) && !LegalOperations && !ST->isVolatile()) ||<br>
+ TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i32)) {<br>
+ ;<br></blockquote><div><br></div><div>Why is this still here?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ Tmp = DAG.getConstant((uint32_t)CFP->getValueAPF().<br>
+ bitcastToAPInt().getZExtValue(), SDLoc(CFP),<br>
+ MVT::i32);<br>
+ SDValue NewSt = DAG.getStore(Chain, DL, Tmp,<br>
+ Ptr, ST->getMemOperand());<br>
+<br>
+ dbgs() << "Replacing FP constant: ";<br></blockquote><div><br></div><div>No no no... This is spewing all over my builds now...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ Value->dump(&DAG);<br>
+<br>
+ if (cast<StoreSDNode>(NewSt)->getMemoryVT() != MVT::i32) {<br>
+ dbgs() << "Different memoryvt\n";<br></blockquote><div><br></div><div>Along with all of this.</div><div><br></div><div>Was this really ready to be committed?</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ } else {<br>
+ dbgs() << "same memoryvt\n";<br>
+ }<br>
+<br>
+<br>
+ return NewSt;<br>
+ }<br>
+<br>
+ return SDValue();<br>
+ case MVT::f64:<br>
+ if ((TLI.isTypeLegal(MVT::i64) && !LegalOperations &&<br>
+ !ST->isVolatile()) ||<br>
+ TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i64)) {<br>
+ ;<br>
+ Tmp = DAG.getConstant(CFP->getValueAPF().bitcastToAPInt().<br>
+ getZExtValue(), SDLoc(CFP), MVT::i64);<br>
+ return DAG.getStore(Chain, DL, Tmp,<br>
+ Ptr, ST->getMemOperand());<br>
+ }<br>
+<br>
+ if (!ST->isVolatile() &&<br>
+ TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i32)) {<br>
+ // Many FP stores are not made apparent until after legalize, e.g. for<br>
+ // argument passing. Since this is so common, custom legalize the<br>
+ // 64-bit integer store into two 32-bit stores.<br>
+ uint64_t Val = CFP->getValueAPF().bitcastToAPInt().getZExtValue();<br>
+ SDValue Lo = DAG.getConstant(Val & 0xFFFFFFFF, SDLoc(CFP), MVT::i32);<br>
+ SDValue Hi = DAG.getConstant(Val >> 32, SDLoc(CFP), MVT::i32);<br>
+ if (DAG.getDataLayout().isBigEndian())<br>
+ std::swap(Lo, Hi);<br>
+<br>
+ unsigned Alignment = ST->getAlignment();<br>
+ bool isVolatile = ST->isVolatile();<br>
+ bool isNonTemporal = ST->isNonTemporal();<br>
+ AAMDNodes AAInfo = ST->getAAInfo();<br>
+<br>
+ SDValue St0 = DAG.getStore(Chain, DL, Lo,<br>
+ Ptr, ST->getPointerInfo(),<br>
+ isVolatile, isNonTemporal,<br>
+ ST->getAlignment(), AAInfo);<br>
+ Ptr = DAG.getNode(ISD::ADD, DL, Ptr.getValueType(), Ptr,<br>
+ DAG.getConstant(4, DL, Ptr.getValueType()));<br>
+ Alignment = MinAlign(Alignment, 4U);<br>
+ SDValue St1 = DAG.getStore(Chain, DL, Hi,<br>
+ Ptr, ST->getPointerInfo().getWithOffset(4),<br>
+ isVolatile, isNonTemporal,<br>
+ Alignment, AAInfo);<br>
+ return DAG.getNode(ISD::TokenFactor, DL, MVT::Other,<br>
+ St0, St1);<br>
+ }<br>
+<br>
+ return SDValue();<br>
+ }<br>
+}<br>
+<br>
SDValue DAGCombiner::visitSTORE(SDNode *N) {<br>
StoreSDNode *ST = cast<StoreSDNode>(N);<br>
SDValue Chain = ST->getChain();<br>
@@ -11389,78 +11486,13 @@ SDValue DAGCombiner::visitSTORE(SDNode *<br>
return Chain;<br>
<br>
// Turn 'store float 1.0, Ptr' -> 'store int 0x12345678, Ptr'<br>
- if (ConstantFPSDNode *CFP = dyn_cast<ConstantFPSDNode>(Value)) {<br>
- // NOTE: If the original store is volatile, this transform must not increase<br>
- // the number of stores. For example, on x86-32 an f64 can be stored in one<br>
- // processor operation but an i64 (which is not legal) requires two. So the<br>
- // transform should not be done in this case.<br>
- if (Value.getOpcode() != ISD::TargetConstantFP) {<br>
- SDValue Tmp;<br>
- switch (CFP->getSimpleValueType(0).SimpleTy) {<br>
- default: llvm_unreachable("Unknown FP type");<br>
- case MVT::f16: // We don't do this for these yet.<br>
- case MVT::f80:<br>
- case MVT::f128:<br>
- case MVT::ppcf128:<br>
- break;<br>
- case MVT::f32:<br>
- if ((isTypeLegal(MVT::i32) && !LegalOperations && !ST->isVolatile()) ||<br>
- TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i32)) {<br>
- ;<br>
- Tmp = DAG.getConstant((uint32_t)CFP->getValueAPF().<br>
- bitcastToAPInt().getZExtValue(), SDLoc(CFP),<br>
- MVT::i32);<br>
- return DAG.getStore(Chain, SDLoc(N), Tmp,<br>
- Ptr, ST->getMemOperand());<br>
- }<br>
- break;<br>
- case MVT::f64:<br>
- if ((TLI.isTypeLegal(MVT::i64) && !LegalOperations &&<br>
- !ST->isVolatile()) ||<br>
- TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i64)) {<br>
- ;<br>
- Tmp = DAG.getConstant(CFP->getValueAPF().bitcastToAPInt().<br>
- getZExtValue(), SDLoc(CFP), MVT::i64);<br>
- return DAG.getStore(Chain, SDLoc(N), Tmp,<br>
- Ptr, ST->getMemOperand());<br>
- }<br>
-<br>
- if (!ST->isVolatile() &&<br>
- TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i32)) {<br>
- // Many FP stores are not made apparent until after legalize, e.g. for<br>
- // argument passing. Since this is so common, custom legalize the<br>
- // 64-bit integer store into two 32-bit stores.<br>
- uint64_t Val = CFP->getValueAPF().bitcastToAPInt().getZExtValue();<br>
- SDValue Lo = DAG.getConstant(Val & 0xFFFFFFFF, SDLoc(CFP), MVT::i32);<br>
- SDValue Hi = DAG.getConstant(Val >> 32, SDLoc(CFP), MVT::i32);<br>
- if (DAG.getDataLayout().isBigEndian())<br>
- std::swap(Lo, Hi);<br>
-<br>
- unsigned Alignment = ST->getAlignment();<br>
- bool isVolatile = ST->isVolatile();<br>
- bool isNonTemporal = ST->isNonTemporal();<br>
- AAMDNodes AAInfo = ST->getAAInfo();<br>
-<br>
- SDLoc DL(N);<br>
-<br>
- SDValue St0 = DAG.getStore(Chain, SDLoc(ST), Lo,<br>
- Ptr, ST->getPointerInfo(),<br>
- isVolatile, isNonTemporal,<br>
- ST->getAlignment(), AAInfo);<br>
- Ptr = DAG.getNode(ISD::ADD, DL, Ptr.getValueType(), Ptr,<br>
- DAG.getConstant(4, DL, Ptr.getValueType()));<br>
- Alignment = MinAlign(Alignment, 4U);<br>
- SDValue St1 = DAG.getStore(Chain, SDLoc(ST), Hi,<br>
- Ptr, ST->getPointerInfo().getWithOffset(4),<br>
- isVolatile, isNonTemporal,<br>
- Alignment, AAInfo);<br>
- return DAG.getNode(ISD::TokenFactor, DL, MVT::Other,<br>
- St0, St1);<br>
- }<br>
-<br>
- break;<br>
- }<br>
- }<br>
+ //<br>
+ // Make sure to do this only after attempting to merge stores in order to<br>
+ // avoid changing the types of some subset of stores due to visit order,<br>
+ // preventing their merging.<br>
+ if (isa<ConstantFPSDNode>(Value)) {<br>
+ if (SDValue NewSt = replaceStoreOfFPConstant(ST))<br>
+ return NewSt;<br>
}<br>
<br>
// Try to infer better alignment information than the store already has.<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>