<div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote">On Fri, Aug 14, 2015 at 2:11 AM, James Molloy <span dir="ltr"><<a href="mailto:James.Molloy@arm.com" target="_blank">James.Molloy@arm.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">



<div style="word-wrap:break-word">
Hi Ahmed,
<div><br>
</div>
<div>Nice catch, thanks!</div>
<div><br>
</div>
<div>I’ve conditionalised this for non-f16 in r245035 and added a test.</div>
<div><br>
</div>
<div>fmin/fmax won’t be generated for f16 now and won’t currently be when D12015 lands, as D12015 checks that fmin/fmax is legal or custom - it doesn’t handle promoted types. This is an obvious improvement I could make.</div></div></blockquote><div><br></div><div>Ah, right.  Thanks for the fix!</div><div><br></div><div>-Ahmed</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">
<div>Cheers,</div>
<div><br>
</div>
<div>James</div><div><div class="h5">
<div><br>
<div>
<div>
<blockquote type="cite">
<div>On 13 Aug 2015, at 23:32, Ahmed Bougacha <<a href="mailto:ahmed.bougacha@gmail.com" target="_blank">ahmed.bougacha@gmail.com</a>> wrote:</div>
<br>
<div><br>
<span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">On
 Tue, Aug 11, 2015 at 5:06 AM, James Molloy via llvm-commits<span> </span></span><span dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important"></span><span style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;float:none;display:inline!important">wrote:</span><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<blockquote class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
Author: jamesm<br>
Date: Tue Aug 11 07:06:33 2015<br>
New Revision: 244594<br>
<br>
URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=244594&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=244594&view=rev</a><br>
Log:<br>
[AArch64] Replace the custom AArch64ISD::FMIN/MAX nodes with ISD::FMINNAN/MAXNAN<br>
<br>
NFCI. This just removes custom ISDNodes that are no longer needed.<br>
<br>
Modified:<br>
   <span> </span>llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp<br>
   <span> </span>llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h<br>
   <span> </span>llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td<br>
<br>
Modified: llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp<br>
URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp?rev=244594&r1=244593&r2=244594&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp?rev=244594&r1=244593&r2=244594&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp (original)<br>
+++ llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.cpp Tue Aug 11 07:06:33 2015<br>
@@ -679,6 +679,11 @@ void AArch64TargetLowering::addTypeForNE<br>
                             ISD::SABSDIFF, ISD::UABSDIFF})<br>
       setOperationAction(Opcode, VT.getSimpleVT(), Legal);<br>
<br>
+  // F[MIN|MAX]NAN are available for all FP NEON types.<br>
+  if (VT.isFloatingPoint())<br>
+    for (unsigned Opcode : {ISD::FMINNAN, ISD::FMAXNAN})<br>
+      setOperationAction(Opcode, VT.getSimpleVT(), Legal);<br>
+<br>
</blockquote>
<div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<br>
</div>
<div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
f16 shouldn't be Legal (the Expand default is fine, but Promote with the other f16 setOperationActions is better).</div>
<div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<br>
</div>
<div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
I guess this can't fire without D12015;  when you land that, can you fix this and add an f{min,max}nan testcase to f16-instructions.ll (or elsewhere)?</div>
<div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<br>
</div>
<div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
Thanks!</div>
<div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<br>
</div>
<div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
-Ahmed</div>
<div style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
 </div>
<blockquote class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
   if (Subtarget->isLittleEndian()) {<br>
     for (unsigned im = (unsigned)ISD::PRE_INC;<br>
         <span> </span>im != (unsigned)ISD::LAST_INDEXED_MODE; ++im) {<br>
@@ -818,8 +823,6 @@ const char *AArch64TargetLowering::getTa<br>
   case AArch64ISD::CCMN:              return "AArch64ISD::CCMN";<br>
   case AArch64ISD::FCCMP:             return "AArch64ISD::FCCMP";<br>
   case AArch64ISD::FCMP:              return "AArch64ISD::FCMP";<br>
-  case AArch64ISD::FMIN:              return "AArch64ISD::FMIN";<br>
-  case AArch64ISD::FMAX:              return "AArch64ISD::FMAX";<br>
   case AArch64ISD::DUP:               return "AArch64ISD::DUP";<br>
   case AArch64ISD::DUPLANE8:          return "AArch64ISD::DUPLANE8";<br>
   case AArch64ISD::DUPLANE16:         return "AArch64ISD::DUPLANE16";<br>
@@ -8219,10 +8222,10 @@ static SDValue performIntrinsicCombine(S<br>
   case Intrinsic::aarch64_neon_umaxv:<br>
     return combineAcrossLanesIntrinsic(AArch64ISD::UMAXV, N, DAG);<br>
   case Intrinsic::aarch64_neon_fmax:<br>
-    return DAG.getNode(AArch64ISD::FMAX, SDLoc(N), N->getValueType(0),<br>
+    return DAG.getNode(ISD::FMAXNAN, SDLoc(N), N->getValueType(0),<br>
                       <span> </span>N->getOperand(1), N->getOperand(2));<br>
   case Intrinsic::aarch64_neon_fmin:<br>
-    return DAG.getNode(AArch64ISD::FMIN, SDLoc(N), N->getValueType(0),<br>
+    return DAG.getNode(ISD::FMINNAN, SDLoc(N), N->getValueType(0),<br>
                       <span> </span>N->getOperand(1), N->getOperand(2));<br>
   case Intrinsic::aarch64_neon_sabd:<br>
     return DAG.getNode(ISD::SABSDIFF, SDLoc(N), N->getValueType(0),<br>
@@ -9147,7 +9150,7 @@ static SDValue performSelectCCCombine(SD<br>
   case ISD::SETLT:<br>
   case ISD::SETLE:<br>
     IsOrEqual = (CC == ISD::SETLE || CC == ISD::SETOLE || CC == ISD::SETULE);<br>
-    Opcode = IsReversed ? AArch64ISD::FMAX : AArch64ISD::FMIN;<br>
+    Opcode = IsReversed ? ISD::FMAXNAN : ISD::FMINNAN;<br>
     break;<br>
<br>
   case ISD::SETUGT:<br>
@@ -9158,7 +9161,7 @@ static SDValue performSelectCCCombine(SD<br>
   case ISD::SETGT:<br>
   case ISD::SETGE:<br>
     IsOrEqual = (CC == ISD::SETGE || CC == ISD::SETOGE || CC == ISD::SETUGE);<br>
-    Opcode = IsReversed ? AArch64ISD::FMIN : AArch64ISD::FMAX;<br>
+    Opcode = IsReversed ? ISD::FMINNAN : ISD::FMAXNAN;<br>
     break;<br>
   }<br>
<br>
<br>
Modified: llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h<br>
URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h?rev=244594&r1=244593&r2=244594&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h?rev=244594&r1=244593&r2=244594&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h (original)<br>
+++ llvm/trunk/lib/Target/AArch64/AArch64ISelLowering.h Tue Aug 11 07:06:33 2015<br>
@@ -66,10 +66,6 @@ enum NodeType : unsigned {<br>
   // Floating point comparison<br>
   FCMP,<br>
<br>
-  // Floating point max and min instructions.<br>
-  FMAX,<br>
-  FMIN,<br>
-<br>
   // Scalar extract<br>
   EXTR,<br>
<br>
<br>
Modified: llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td<br>
URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td?rev=244594&r1=244593&r2=244594&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td?rev=244594&r1=244593&r2=244594&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td (original)<br>
+++ llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.td Tue Aug 11 07:06:33 2015<br>
@@ -182,9 +182,6 @@ def AArch64threadpointer : SDNode<"AArch<br>
<br>
 def AArch64fcmp      : SDNode<"AArch64ISD::FCMP", SDT_AArch64FCmp>;<br>
<br>
-def AArch64fmax      : SDNode<"AArch64ISD::FMAX", SDTFPBinOp>;<br>
-def AArch64fmin      : SDNode<"AArch64ISD::FMIN", SDTFPBinOp>;<br>
-<br>
 def AArch64dup       : SDNode<"AArch64ISD::DUP", SDT_AArch64Dup>;<br>
 def AArch64duplane8  : SDNode<"AArch64ISD::DUPLANE8", SDT_AArch64DupLane>;<br>
 def AArch64duplane16 : SDNode<"AArch64ISD::DUPLANE16", SDT_AArch64DupLane>;<br>
@@ -2506,18 +2503,18 @@ let SchedRW = [WriteFDiv] in {<br>
 defm FDIV   : TwoOperandFPData<0b0001, "fdiv", fdiv>;<br>
 }<br>
 defm FMAXNM : TwoOperandFPData<0b0110, "fmaxnm", int_aarch64_neon_fmaxnm>;<br>
-defm FMAX   : TwoOperandFPData<0b0100, "fmax", AArch64fmax>;<br>
+defm FMAX   : TwoOperandFPData<0b0100, "fmax", fmaxnan>;<br>
 defm FMINNM : TwoOperandFPData<0b0111, "fminnm", int_aarch64_neon_fminnm>;<br>
-defm FMIN   : TwoOperandFPData<0b0101, "fmin", AArch64fmin>;<br>
+defm FMIN   : TwoOperandFPData<0b0101, "fmin", fminnan>;<br>
 let SchedRW = [WriteFMul] in {<br>
 defm FMUL   : TwoOperandFPData<0b0000, "fmul", fmul>;<br>
 defm FNMUL  : TwoOperandFPDataNeg<0b1000, "fnmul", fmul>;<br>
 }<br>
 defm FSUB   : TwoOperandFPData<0b0011, "fsub", fsub>;<br>
<br>
-def : Pat<(v1f64 (AArch64fmax (v1f64 FPR64:$Rn), (v1f64 FPR64:$Rm))),<br>
+def : Pat<(v1f64 (fmaxnan (v1f64 FPR64:$Rn), (v1f64 FPR64:$Rm))),<br>
           (FMAXDrr FPR64:$Rn, FPR64:$Rm)>;<br>
-def : Pat<(v1f64 (AArch64fmin (v1f64 FPR64:$Rn), (v1f64 FPR64:$Rm))),<br>
+def : Pat<(v1f64 (fminnan (v1f64 FPR64:$Rn), (v1f64 FPR64:$Rm))),<br>
           (FMINDrr FPR64:$Rn, FPR64:$Rm)>;<br>
 def : Pat<(v1f64 (int_aarch64_neon_fmaxnm (v1f64 FPR64:$Rn), (v1f64 FPR64:$Rm))),<br>
           (FMAXNMDrr FPR64:$Rn, FPR64:$Rm)>;<br>
@@ -2809,11 +2806,11 @@ defm FDIV    : SIMDThreeSameVectorFP<1,0<br>
 defm FMAXNMP : SIMDThreeSameVectorFP<1,0,0b11000,"fmaxnmp", int_aarch64_neon_fmaxnmp>;<br>
 defm FMAXNM  : SIMDThreeSameVectorFP<0,0,0b11000,"fmaxnm", int_aarch64_neon_fmaxnm>;<br>
 defm FMAXP   : SIMDThreeSameVectorFP<1,0,0b11110,"fmaxp", int_aarch64_neon_fmaxp>;<br>
-defm FMAX    : SIMDThreeSameVectorFP<0,0,0b11110,"fmax", AArch64fmax>;<br>
+defm FMAX    : SIMDThreeSameVectorFP<0,0,0b11110,"fmax", fmaxnan>;<br>
 defm FMINNMP : SIMDThreeSameVectorFP<1,1,0b11000,"fminnmp", int_aarch64_neon_fminnmp>;<br>
 defm FMINNM  : SIMDThreeSameVectorFP<0,1,0b11000,"fminnm", int_aarch64_neon_fminnm>;<br>
 defm FMINP   : SIMDThreeSameVectorFP<1,1,0b11110,"fminp", int_aarch64_neon_fminp>;<br>
-defm FMIN    : SIMDThreeSameVectorFP<0,1,0b11110,"fmin", AArch64fmin>;<br>
+defm FMIN    : SIMDThreeSameVectorFP<0,1,0b11110,"fmin", fminnan>;<br>
<br>
 // NOTE: The operands of the PatFrag are reordered on FMLA/FMLS because the<br>
 // instruction expects the addend first, while the fma intrinsic puts it last.<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></blockquote>
</div>
</blockquote>
</div>
<br>
</div>
</div>
<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>