[PATCH] D87571: [DAGCombiner] Fold fmin/fmax with INF

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14080-14082
   if (isConstantFPBuildVectorOrConstantFP(N0) &&
       !isConstantFPBuildVectorOrConstantFP(N1))
     return DAG.getNode(N->getOpcode(), SDLoc(N), VT, N1, N0);
----------------
There's a bug here that should be addressed first (probably lacking test coverage) - we dropped the FMF when creating the new node.
It would be more efficient to std::swap(N0, N1) and just use those local names in all of the later code?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14099-14100
+      // maxnum(X, +inf) -> +inf
+      // maximum(X, -inf) -> -inf if nnan
+      // minimum(X, +inf) -> +inf if nnan
+      if (IsMin == AF.isNegative() && (!PropagatesNaN || Flags.hasNoNaNs()))
----------------
If we're using the same order of functions as in the code comment above here, the max/min names are inverted.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14107
+      // minimum(X, +inf) -> X
+      // maximum(X, +inf) -> X
+      if (IsMin != AF.isNegative() && (PropagatesNaN || Flags.hasNoNaNs()))
----------------
"+inf" should be "-inf" on this line?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87571/new/

https://reviews.llvm.org/D87571