Skip to content

Conversation

@actinks
Copy link
Contributor

@actinks actinks commented Jun 6, 2025

FCmp's commutativity predicates do not work with min/max semantics

Closes #142711

FCmp's equality and commutativity predicates do not work with min/max semantics

Closes llvm#142711
@actinks actinks requested a review from nikic as a code owner June 6, 2025 14:00
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Jun 6, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 6, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Acthink Yang (Acthink-Yang)

Changes

FCmp's equality and commutativity predicates do not work with min/max semantics

Closes #142711


Full diff: https://github.com/llvm/llvm-project/pull/143144.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstructionCombining.cpp (+2-1)
  • (modified) llvm/test/Transforms/InstCombine/fcmp-select.ll (+11)
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index 439a86d951a83..c85abd9b26d61 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1729,7 +1729,8 @@ Instruction *InstCombinerImpl::FoldOpIntoSelect(Instruction &Op, SelectInst *SI,
   if (auto *CI = dyn_cast<FCmpInst>(SI->getCondition())) {
     if (CI->hasOneUse()) {
       Value *Op0 = CI->getOperand(0), *Op1 = CI->getOperand(1);
-      if ((TV == Op0 && FV == Op1) || (FV == Op0 && TV == Op1))
+      if (((TV == Op0 && FV == Op1) || (FV == Op0 && TV == Op1)) &&
+          !CI->isEquality() && !CI->isCommutative())
         return nullptr;
     }
   }
diff --git a/llvm/test/Transforms/InstCombine/fcmp-select.ll b/llvm/test/Transforms/InstCombine/fcmp-select.ll
index 408bc1cdc268f..bf08fbfc27302 100644
--- a/llvm/test/Transforms/InstCombine/fcmp-select.ll
+++ b/llvm/test/Transforms/InstCombine/fcmp-select.ll
@@ -268,3 +268,14 @@ define i1 @test_fcmp_select_var_const_unordered(double %x, double %y) {
   %cmp2 = fcmp ugt double %sel, 0x3E80000000000000
   ret i1 %cmp2
 }
+
+define i1 @test_fcmp_ord_select_fcmp_oeq_var_const(double %x) {
+; CHECK-LABEL:    @test_fcmp_ord_select_fcmp_oeq_var_const(
+; CHECK-NEXT:     [[CMP1:%.*]] = fcmp oeq double [[X:%.*]], 0.000000e+00
+; CHECK-NEXT:     ret i1 [[CMP1]]
+;
+  %cmp1 = fcmp ord double %x, 0.000000e+00
+  %sel = select i1 %cmp1, double %x, double 0.000000e+00
+  %cmp2 = fcmp oeq double %sel, 1.000000e+00
+  ret i1 %cmp2
+}

Co-authored-by: Yingwei Zheng <[email protected]>
Copy link
Member

@dtcxzyw dtcxzyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM w/ a nit.

@dtcxzyw dtcxzyw merged commit ca4dfca into llvm:main Jun 7, 2025
7 checks passed
@actinks actinks deleted the instrcombine-fcmp branch June 7, 2025 06:50
tomtor pushed a commit to tomtor/llvm-project that referenced this pull request Jun 14, 2025
)

FCmp's commutativity predicates do not work with min/max semantics

Closes llvm#142711
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missed Optimization: fold fcmp ord + select before fcmp oeq (x, C) into a single fcmp oeq (x, C)

4 participants