<div dir="ltr"><span style="line-height:normal">On Tue, Aug 18, 2015 at 12:51 PM, James Molloy </span><span dir="ltr" style="line-height:normal"><<a href="mailto:James.Molloy@arm.com" target="_blank">James.Molloy@arm.com</a>></span><span style="line-height:normal"> wrote:</span><br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="auto">
<div>Hi Ahmed,</div>
<div><br>
</div>
<div>I'm afraid I'm not quite certain what you mean - could you elaborate?</div></div></blockquote><div><br></div><div><span style="line-height:normal">I was thinking of something like:</span><br></div><div><br></div><div><div><div><div><div><div><div>define double @f(float %a, float %b) #0 {</div><div> %cc = fcmp olt float %a, %b</div><div> %xa = fpext float %a to double</div><div> %xb = fpext float %b to double</div><div> %r = select i1 %cc, double %xa, double %xb</div><div> ret double %r</div><div>}</div><div><br></div><div>attributes #0 = { "no-infs-fp-math"="true" "no-nans-fp-math"="true" "unsafe-fp-math"="true" }</div></div></div></div></div></div></div><div><br></div><div>for which the previous lowering would have generated something like:</div><div><br></div><div> fcvt d0, s0</div><div> fcvt d1, s1</div><div> fmin d0, d0, d1</div><div><br></div><div>But we now generate:</div><div><br></div><div><div><div><div> fcvt d2, s0</div><div> fcvt d3, s1</div><div> fcmp s0, s1</div><div> fcsel d0, d2, d3, lt</div><div><br></div></div></div></div><div>For float/double, it's not a problem, because t<span style="line-height:normal">he now-canonical IR, per InstCombine, would be:</span></div><div><br></div><div><div>define double @f(float %a, float %b) #0 {</div><div> %cc = fcmp olt float %a, %b</div><div> %r.v = select i1 %cc, float %a, float %b</div><div> %r = fpext float %r.v to double</div><div> ret double %r</div><div>}</div></div><div><br></div><div>Which we can indeed match. However, for half, fpexts on the fcmp operands are generated by the legalizer, so neither instcombine nor the builder can match those anymore. We basically have a DAG for:</div><div><br></div><div>define double @f(half %a, half %b) #0 {</div><div> %sa = fpext half %a to float</div><div> %sb = fpext half %b to float</div><div> %cc = fcmp olt float %sa, %sb</div><div> %xa = fpext float %sa to double</div><div> %xb = fpext float %sb to double</div><div> %r = select i1 %cc, double %xa, double %xb</div><div> ret double %r</div><div>}</div><div><br></div><div>Perhaps we want a similar DAG combine, or even have the builder match the fpext on the select result directly? Not sure either's a good idea.</div><div><br></div><div>Again, not a big problem by any means, just wondering if we can/should improve this somehow!</div><div><span style="line-height:normal"><br></span></div><div><span style="line-height:normal">-Ahmed</span></div><div><span style="line-height:normal"> </span><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto">
<div>Cheers,</div>
<div><br>
</div>
<div>James</div><div><div>
<div><br>
<br>
</div>
<div><br>
On 18 Aug 2015, at 19:36, Ahmed Bougacha <<a href="mailto:ahmed.bougacha@gmail.com" target="_blank">ahmed.bougacha@gmail.com</a>> wrote:<br>
<br>
</div>
<blockquote type="cite">
<div>
<div dir="ltr">Hey James,
<div><br>
</div>
<div>If I read this correctly, we can't fold fpext anymore, right? <span style="line-height:normal">Not a big deal right now, as the main case where I see this being useful is a promoted f16 min/max, and that's not even caught currently.</span></div>
<div><span style="line-height:normal">Do you think we should do something similar in the builder? If so, let's file a PR!</span></div>
</div>
<div class="gmail_extra"><br clear="all">
<div>
<div>-Ahmed</div>
</div>
<br>
<div class="gmail_quote">On Mon, Aug 17, 2015 at 12:13 AM, James Molloy via llvm-commits
<span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: jamesm<br>
Date: Mon Aug 17 02:13:20 2015<br>
New Revision: 245198<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=245198&view=rev" rel="noreferrer" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=245198&view=rev</a><br>
Log:<br>
Remove hand-rolled matching for fmin and fmax.<br>
<br>
SDAGBuilder now does this all for us.<br>
<br>
Modified:<br>
llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp<br>
<br>
Modified: llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp?rev=245198&r1=245197&r2=245198&view=diff" rel="noreferrer" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp?rev=245198&r1=245197&r2=245198&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp (original)<br>
+++ llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp Mon Aug 17 02:13:20 2015<br>
@@ -393,6 +393,8 @@ AArch64TargetLowering::AArch64TargetLowe<br>
setOperationAction(ISD::FROUND, Ty, Legal);<br>
setOperationAction(ISD::FMINNUM, Ty, Legal);<br>
setOperationAction(ISD::FMAXNUM, Ty, Legal);<br>
+ setOperationAction(ISD::FMINNAN, Ty, Legal);<br>
+ setOperationAction(ISD::FMAXNAN, Ty, Legal);<br>
}<br>
<br>
setOperationAction(ISD::PREFETCH, MVT::Other, Custom);<br>
@@ -484,7 +486,6 @@ AArch64TargetLowering::AArch64TargetLowe<br>
<br>
setTargetDAGCombine(ISD::SELECT);<br>
setTargetDAGCombine(ISD::VSELECT);<br>
- setTargetDAGCombine(ISD::SELECT_CC);<br>
<br>
setTargetDAGCombine(ISD::INTRINSIC_VOID);<br>
setTargetDAGCombine(ISD::INTRINSIC_W_CHAIN);<br>
@@ -3826,32 +3827,6 @@ SDValue AArch64TargetLowering::LowerSETC<br>
}<br>
}<br>
<br>
-/// A SELECT_CC operation is really some kind of max or min if both values being<br>
-/// compared are, in some sense, equal to the results in either case. However,<br>
-/// it is permissible to compare f32 values and produce directly extended f64<br>
-/// values.<br>
-///<br>
-/// Extending the comparison operands would also be allowed, but is less likely<br>
-/// to happen in practice since their use is right here. Note that truncate<br>
-/// operations would *not* be semantically equivalent.<br>
-static bool selectCCOpsAreFMaxCompatible(SDValue Cmp, SDValue Result) {<br>
- if (Cmp == Result)<br>
- return (Cmp.getValueType() == MVT::f32 ||<br>
- Cmp.getValueType() == MVT::f64);<br>
-<br>
- ConstantFPSDNode *CCmp = dyn_cast<ConstantFPSDNode>(Cmp);<br>
- ConstantFPSDNode *CResult = dyn_cast<ConstantFPSDNode>(Result);<br>
- if (CCmp && CResult && Cmp.getValueType() == MVT::f32 &&<br>
- Result.getValueType() == MVT::f64) {<br>
- bool Lossy;<br>
- APFloat CmpVal = CCmp->getValueAPF();<br>
- CmpVal.convert(APFloat::IEEEdouble, APFloat::rmNearestTiesToEven, &Lossy);<br>
- return CResult->getValueAPF().bitwiseIsEqual(CmpVal);<br>
- }<br>
-<br>
- return Result->getOpcode() == ISD::FP_EXTEND && Result->getOperand(0) == Cmp;<br>
-}<br>
-<br>
SDValue AArch64TargetLowering::LowerSELECT_CC(ISD::CondCode CC, SDValue LHS,<br>
SDValue RHS, SDValue TVal,<br>
SDValue FVal, SDLoc dl,<br>
@@ -9124,75 +9099,6 @@ static SDValue performSelectCombine(SDNo<br>
return DAG.getSelect(DL, ResVT, Mask, N->getOperand(1), N->getOperand(2));<br>
}<br>
<br>
-/// performSelectCCCombine - Target-specific DAG combining for ISD::SELECT_CC<br>
-/// to match FMIN/FMAX patterns.<br>
-static SDValue performSelectCCCombine(SDNode *N, SelectionDAG &DAG) {<br>
- // Try to use FMIN/FMAX instructions for FP selects like "x < y ? x : y".<br>
- // Unless the NoNaNsFPMath option is set, be careful about NaNs:<br>
- // vmax/vmin return NaN if either operand is a NaN;<br>
- // only do the transformation when it matches that behavior.<br>
-<br>
- SDValue CondLHS = N->getOperand(0);<br>
- SDValue CondRHS = N->getOperand(1);<br>
- SDValue LHS = N->getOperand(2);<br>
- SDValue RHS = N->getOperand(3);<br>
- ISD::CondCode CC = cast<CondCodeSDNode>(N->getOperand(4))->get();<br>
-<br>
- unsigned Opcode;<br>
- bool IsReversed;<br>
- if (selectCCOpsAreFMaxCompatible(CondLHS, LHS) &&<br>
- selectCCOpsAreFMaxCompatible(CondRHS, RHS)) {<br>
- IsReversed = false; // x CC y ? x : y<br>
- } else if (selectCCOpsAreFMaxCompatible(CondRHS, LHS) &&<br>
- selectCCOpsAreFMaxCompatible(CondLHS, RHS)) {<br>
- IsReversed = true ; // x CC y ? y : x<br>
- } else {<br>
- return SDValue();<br>
- }<br>
-<br>
- bool IsUnordered = false, IsOrEqual;<br>
- switch (CC) {<br>
- default:<br>
- return SDValue();<br>
- case ISD::SETULT:<br>
- case ISD::SETULE:<br>
- IsUnordered = true;<br>
- case ISD::SETOLT:<br>
- case ISD::SETOLE:<br>
- case ISD::SETLT:<br>
- case ISD::SETLE:<br>
- IsOrEqual = (CC == ISD::SETLE || CC == ISD::SETOLE || CC == ISD::SETULE);<br>
- Opcode = IsReversed ? ISD::FMAXNAN : ISD::FMINNAN;<br>
- break;<br>
-<br>
- case ISD::SETUGT:<br>
- case ISD::SETUGE:<br>
- IsUnordered = true;<br>
- case ISD::SETOGT:<br>
- case ISD::SETOGE:<br>
- case ISD::SETGT:<br>
- case ISD::SETGE:<br>
- IsOrEqual = (CC == ISD::SETGE || CC == ISD::SETOGE || CC == ISD::SETUGE);<br>
- Opcode = IsReversed ? ISD::FMINNAN : ISD::FMAXNAN;<br>
- break;<br>
- }<br>
-<br>
- // If LHS is NaN, an ordered comparison will be false and the result will be<br>
- // the RHS, but FMIN(NaN, RHS) = FMAX(NaN, RHS) = NaN. Avoid this by checking<br>
- // that LHS != NaN. Likewise, for unordered comparisons, check for RHS != NaN.<br>
- if (!DAG.isKnownNeverNaN(IsUnordered ? RHS : LHS))<br>
- return SDValue();<br>
-<br>
- // For xxx-or-equal comparisons, "+0 <= -0" and "-0 >= +0" will both be true,<br>
- // but FMIN will return -0, and FMAX will return +0. So FMIN/FMAX can only be<br>
- // used for unsafe math or if one of the operands is known to be nonzero.<br>
- if (IsOrEqual && !DAG.getTarget().Options.UnsafeFPMath &&<br>
- !(DAG.isKnownNeverZero(LHS) || DAG.isKnownNeverZero(RHS)))<br>
- return SDValue();<br>
-<br>
- return DAG.getNode(Opcode, SDLoc(N), N->getValueType(0), LHS, RHS);<br>
-}<br>
-<br>
/// Get rid of unnecessary NVCASTs (that don't change the type).<br>
static SDValue performNVCASTCombine(SDNode *N) {<br>
if (N->getValueType(0) == N->getOperand(0).getValueType())<br>
@@ -9233,8 +9139,6 @@ SDValue AArch64TargetLowering::PerformDA<br>
return performSelectCombine(N, DCI);<br>
case ISD::VSELECT:<br>
return performVSelectCombine(N, DCI.DAG);<br>
- case ISD::SELECT_CC:<br>
- return performSelectCCCombine(N, DCI.DAG);<br>
case ISD::STORE:<br>
return performSTORECombine(N, DCI, DAG, Subtarget);<br>
case AArch64ISD::BRCOND:<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>
<br>
</div>
</div>
</blockquote>
<br>
</div></div><font face="Arial" color="Black" size="2">-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents
to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.<br>
<br>
ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590<br>
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782<br>
</font>
</div>
</blockquote></div><br></div></div>