Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[InstCombine] Fold fcmp into select #86482

Merged
merged 4 commits into from
Apr 23, 2024
Merged

Conversation

dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Mar 25, 2024

This patch simplifies fcmp (select Cond, C1, C2), C3 patterns in ceres:
Alive2: https://alive2.llvm.org/ce/z/fWh_sD

define i1 @src(double %x) {
  %cmp1 = fcmp ord double %x, 0.000000e+00
  %sel = select i1 %cmp1, double 0xFFFFFFFFFFFFFFFF, double 0.000000e+00
  %cmp2 = fcmp oeq double %sel, 0.000000e+00
  ret i1 %cmp2
}

define i1 @tgt(double %x) {
  %cmp1 = fcmp uno double %x, 0.000000e+00
  ret i1 %cmp1
}

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 25, 2024

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

This patch simplifies fcmp (select Cond, C1, C2), C3 patterns in ceres:
Alive2: https://alive2.llvm.org/ce/z/fWh_sD

define i1 @<!-- -->src(double %x) {
  %cmp1 = fcmp ord double %x, 0.000000e+00
  %sel = select i1 %cmp1, double 0xFFFFFFFFFFFFFFFF, double 0.000000e+00
  %cmp2 = fcmp oeq double %sel, 0.000000e+00
  ret i1 %cmp2
}

define i1 @<!-- -->tgt(double %x) {
  %cmp1 = fcmp uno double %x, 0.000000e+00
  ret i1 %cmp1
}


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

3 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+4)
  • (modified) llvm/test/Transforms/InstCombine/fcmp-select.ll (+65)
  • (modified) llvm/test/Transforms/InstCombine/select-select.ll (+3-4)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index db302d7e526844..ba13c59e047b52 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -8020,6 +8020,10 @@ Instruction *InstCombinerImpl::visitFCmpInst(FCmpInst &I) {
       if (Instruction *NV = foldOpIntoPhi(I, cast<PHINode>(LHSI)))
         return NV;
       break;
+    case Instruction::Select:
+      if (Instruction *NV = FoldOpIntoSelect(I, cast<SelectInst>(LHSI)))
+        return NV;
+      break;
     case Instruction::SIToFP:
     case Instruction::UIToFP:
       if (Instruction *NV = foldFCmpIntToFPConst(I, LHSI, RHSC))
diff --git a/llvm/test/Transforms/InstCombine/fcmp-select.ll b/llvm/test/Transforms/InstCombine/fcmp-select.ll
index f37c586845b12d..62fad8349d22e2 100644
--- a/llvm/test/Transforms/InstCombine/fcmp-select.ll
+++ b/llvm/test/Transforms/InstCombine/fcmp-select.ll
@@ -148,3 +148,68 @@ define i1 @fcmp_ogt_select(i1 %cond, float %a, float %b) {
   %res = fcmp ogt float %lhs, %rhs
   ret i1 %res
 }
+
+define i1 @test_fcmp_select_const_const(double %x) {
+; CHECK-LABEL: @test_fcmp_select_const_const(
+; CHECK-NEXT:    [[CMP1:%.*]] = fcmp uno double [[X:%.*]], 0.000000e+00
+; CHECK-NEXT:    ret i1 [[CMP1]]
+;
+  %cmp1 = fcmp ord double %x, 0.000000e+00
+  %sel = select i1 %cmp1, double 0xFFFFFFFFFFFFFFFF, double 0.000000e+00
+  %cmp2 = fcmp oeq double %sel, 0.000000e+00
+  ret i1 %cmp2
+}
+
+define i1 @test_fcmp_select_var_const(double %x, double %y) {
+; CHECK-LABEL: @test_fcmp_select_var_const(
+; CHECK-NEXT:    [[CMP1:%.*]] = fcmp ule double [[X:%.*]], 0x3E80000000000000
+; CHECK-NEXT:    [[TMP1:%.*]] = fcmp olt double [[Y:%.*]], 0x3E80000000000000
+; CHECK-NEXT:    [[CMP2:%.*]] = select i1 [[CMP1]], i1 true, i1 [[TMP1]]
+; CHECK-NEXT:    ret i1 [[CMP2]]
+;
+  %cmp1 = fcmp ogt double %x, 0x3E80000000000000
+  %sel = select i1 %cmp1, double %y, double 0.000000e+00
+  %cmp2 = fcmp olt double %sel, 0x3E80000000000000
+  ret i1 %cmp2
+}
+
+define i1 @test_fcmp_select_var_const_fmf(double %x, double %y) {
+; CHECK-LABEL: @test_fcmp_select_var_const_fmf(
+; CHECK-NEXT:    [[CMP1:%.*]] = fcmp ule double [[X:%.*]], 0x3E80000000000000
+; CHECK-NEXT:    [[CMP2:%.*]] = fcmp nnan olt double [[SEL:%.*]], 0x3E80000000000000
+; CHECK-NEXT:    [[CMP3:%.*]] = select i1 [[CMP1]], i1 true, i1 [[CMP2]]
+; CHECK-NEXT:    ret i1 [[CMP3]]
+;
+  %cmp1 = fcmp ogt double %x, 0x3E80000000000000
+  %sel = select i1 %cmp1, double %y, double 0.000000e+00
+  %cmp2 = fcmp nnan olt double %sel, 0x3E80000000000000
+  ret i1 %cmp2
+}
+
+define <2 x i1> @test_fcmp_select_const_const_vec(<2 x double> %x) {
+; CHECK-LABEL: @test_fcmp_select_const_const_vec(
+; CHECK-NEXT:    [[CMP1:%.*]] = fcmp uno <2 x double> [[X:%.*]], zeroinitializer
+; CHECK-NEXT:    ret <2 x i1> [[CMP1]]
+;
+  %cmp1 = fcmp ord <2 x double> %x, zeroinitializer
+  %sel = select <2 x i1> %cmp1, <2 x double> <double 0xFFFFFFFFFFFFFFFF, double 0xFFFFFFFFFFFFFFFF>, <2 x double> zeroinitializer
+  %cmp2 = fcmp oeq <2 x double> %sel, zeroinitializer
+  ret <2 x i1> %cmp2
+}
+
+; Don't break clamp idioms
+
+define double @test_fcmp_select_clamp(double %x) {
+; CHECK-LABEL: @test_fcmp_select_clamp(
+; CHECK-NEXT:    [[CMP1:%.*]] = fcmp ogt double [[X:%.*]], 9.000000e-01
+; CHECK-NEXT:    [[SEL1:%.*]] = select i1 [[CMP1]], double 9.000000e-01, double [[X]]
+; CHECK-NEXT:    [[CMP2:%.*]] = fcmp olt double [[SEL1]], 5.000000e-01
+; CHECK-NEXT:    [[SEL2:%.*]] = select i1 [[CMP2]], double 5.000000e-01, double [[SEL1]]
+; CHECK-NEXT:    ret double [[SEL2]]
+;
+  %cmp1 = fcmp ogt double %x, 9.000000e-01
+  %sel1 = select i1 %cmp1, double 9.000000e-01, double %x
+  %cmp2 = fcmp olt double %sel1, 5.000000e-01
+  %sel2 = select i1 %cmp2, double 5.000000e-01, double %sel1
+  ret double %sel2
+}
diff --git a/llvm/test/Transforms/InstCombine/select-select.ll b/llvm/test/Transforms/InstCombine/select-select.ll
index 785766136fb71b..84fe973093e327 100644
--- a/llvm/test/Transforms/InstCombine/select-select.ll
+++ b/llvm/test/Transforms/InstCombine/select-select.ll
@@ -18,11 +18,10 @@ define float @foo1(float %a) {
 
 define float @foo2(float %a) {
 ; CHECK-LABEL: @foo2(
-; CHECK-NEXT:    [[B:%.*]] = fcmp ogt float [[A:%.*]], 0.000000e+00
-; CHECK-NEXT:    [[C:%.*]] = select i1 [[B]], float [[A]], float 0.000000e+00
+; CHECK-NEXT:    [[B:%.*]] = fcmp ule float [[C:%.*]], 0.000000e+00
 ; CHECK-NEXT:    [[D:%.*]] = fcmp olt float [[C]], 1.000000e+00
-; CHECK-NEXT:    [[E:%.*]] = select i1 [[B]], float [[A]], float 0.000000e+00
-; CHECK-NEXT:    [[F:%.*]] = select i1 [[D]], float [[E]], float 1.000000e+00
+; CHECK-NEXT:    [[E:%.*]] = select i1 [[D]], float [[C]], float 1.000000e+00
+; CHECK-NEXT:    [[F:%.*]] = select i1 [[B]], float 0.000000e+00, float [[E]]
 ; CHECK-NEXT:    ret float [[F]]
 ;
   %b = fcmp ogt float %a, 0.0

dtcxzyw added a commit to dtcxzyw/llvm-opt-benchmark that referenced this pull request Mar 25, 2024
Copy link

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link

✅ With the latest revision this PR passed the Python code formatter.

case Instruction::Select:
if (Instruction *NV = FoldOpIntoSelect(I, cast<SelectInst>(LHSI)))
return NV;
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

If both arms are constant, should we allow multiuse select?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we can relax it in InstCombinerImpl::FoldOpIntoSelect.

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO here, we have the argument FoldWithMultiUse and it would be a bit confusing if we started to surreptitiously assume it.

@@ -8020,6 +8020,10 @@ Instruction *InstCombinerImpl::visitFCmpInst(FCmpInst &I) {
if (Instruction *NV = foldOpIntoPhi(I, cast<PHINode>(LHSI)))
return NV;
break;
case Instruction::Select:
if (Instruction *NV = FoldOpIntoSelect(I, cast<SelectInst>(LHSI)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we update constantFoldOperationIntoSelectOperand to recognize fcmp?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be done in a separate patch.
Related patch: https://reviews.llvm.org/D146349

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

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

LGTM, but please add a multi-use test. (Without changing your current multi-use handling.)

@dtcxzyw
Copy link
Member Author

dtcxzyw commented Apr 22, 2024

LGTM, but please add a multi-use test. (Without changing your current multi-use handling.)

Done.

@dtcxzyw dtcxzyw merged commit 9fb7a73 into llvm:main Apr 23, 2024
4 checks passed
@dtcxzyw dtcxzyw deleted the perf/fcmp-into-select branch April 23, 2024 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants