[llvm] [InstCombine] Extend fcmp+select folding to minnum/maxnum intrinsics (PR #112088)

Alexey Bader via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 14 09:16:17 PDT 2024


================
@@ -3834,11 +3834,13 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
     // minnum/maxnum intrinsics.
     if (SIFPOp->hasNoNaNs() && SIFPOp->hasNoSignedZeros()) {
       Value *X, *Y;
-      if (match(&SI, m_OrdFMax(m_Value(X), m_Value(Y))))
+      if (match(&SI, m_OrdFMax(m_Value(X), m_Value(Y))) ||
+          match(&SI, m_UnordFMax(m_Value(X), m_Value(Y))))
----------------
bader wrote:

I found 2 more uses of `m_OrdFMax` and both of them are combining `m_OrdFMax` with `m_UnordFMax`. It looks like the InstCombine is the only place where pattern matching is incomplete.

https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/ValueTracking.cpp#L8244-L8263

https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/IVDescriptors.cpp#L695-L696 and 
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/IVDescriptors.cpp#L699-L700.

Based on the description of `m_OrdFMax` and `m_UnodFMax` they must always be applied together. So, in my opinion, it's worth adding `m_OrdOrUnordFMax` and probably remove existing matchers to avoid similar mistakes in the future.

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


More information about the llvm-commits mailing list