<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 class="gmail_signature">-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">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>