[llvm] [InstSimplify] Optimize maximumnum and minimumnum (PR #139581)

[Lewis Crawford via llvm-commits]

================
@@ -6725,32 +6735,50 @@ Value *llvm::simplifyBinaryIntrinsic(Intrinsic::ID IID, Type *ReturnType,
       return Op0;
 
     bool PropagateNaN = IID == Intrinsic::minimum || IID == Intrinsic::maximum;
-    bool IsMin = IID == Intrinsic::minimum || IID == Intrinsic::minnum;
-
-    // minnum(X, nan) -> X
-    // maxnum(X, nan) -> X
-    // minimum(X, nan) -> nan
-    // maximum(X, nan) -> nan
-    if (match(Op1, m_NaN()))
-      return PropagateNaN ? propagateNaN(cast<Constant>(Op1)) : Op0;
+    bool PropagateSNaN = IID == Intrinsic::minnum || IID == Intrinsic::maxnum;
+    bool IsMin = IID == Intrinsic::minimum || IID == Intrinsic::minnum ||
+                 IID == Intrinsic::minimumnum;
+
+    // minnum(x, qnan) -> x
+    // maxnum(x, qnan) -> x
+    // minnum(x, snan) -> qnan
+    // maxnum(x, snan) -> qnan
+    // minimum(X, nan) -> qnan
+    // maximum(X, nan) -> qnan
+    if (PropagateSNaN && match(Op1, m_sNaN())) {
+      return propagateNaN(cast<Constant>(Op1));
+    } else if (match(Op1, m_NaN())) {
+      if (PropagateNaN)
+        return propagateNaN(cast<Constant>(Op1));
+      // In cases like mixed <sNaN, qNaN> vectors, avoid the optimization to
+      // allow correct sNaN propagation where necessary.
+      else if (PropagateSNaN && !match(Op1, m_qNaN()))
+        break;
+      else
+        return Op0;
----------------
LewisCrawford wrote:

I think element-wise comparisons will end up taking this slightly out of the current scope of the patch, as it will introduce cases like: maxnum(<x, y>, <sNaN, qNaN>) -> <qNaN, y> , which will require insert/extract chains to be created, and might end up with cases where there are not clear perf wins from re-creating partially applied vectors where some elements are NaN vs just using maxnum on the original vector despite some elements being NaN. To avoid these partially applied cases, we'd need to check whether the whole vec is sNaN or qNaN (ignoring poison/undef elements), which is what the current patch does with the matchers already.

It might be worth considering these kind of element-wise folds in future (and for partial Inf, partial poison, partial FMAX vectors too in addition to NaNs maybe), but I think that would be better-suited to a separate patch.

https://github.com/llvm/llvm-project/pull/139581