[InstCombine] Extend fcmp+select folding to minnum/maxnum intrinsics #112088
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Today, InstCombine can fold fcmp+select patterns to minnum/maxnum intrinsics
when the nnan and nsz flags are set. The ordering of the operands in both
the fcmp and select instructions is important for the folding to occur.
maxnum patterns:
1. (a op b) ? a : b -> maxnum(a, b), where op is one of {ogt, oge}
2. (a op b) ? b : a -> maxnum(a, b), where op is one of {ule, ult}
The second pattern is supposed to make the order of the operands in the
select instruction irrelevant. However, the pattern matching code uses the
CmpInst::getInversePredicate method to invert the comparison predicate.
This method doesn't take into account the fast-math flags, which can lead
missing the folding opportunity.
The patch extends the pattern matching code to handle unordered fcmp
instructions. This allows the folding to occur even when the select
instruction has the operands in the inverse order.
New maxnum patterns:
1. (a op b) ? a : b -> maxnum(a, b), where op is one of {ugt, uge}
2. (a op b) ? b : a -> maxnum(a, b), where op is one of {ole, olt}
The same changes are applied to the minnum intrinsic.
|
@llvm/pr-subscribers-llvm-ir @llvm/pr-subscribers-llvm-transforms Author: Alexey Bader (bader) ChangesToday, InstCombine can fold fcmp+select patterns to minnum/maxnum intrinsics when the nnan and nsz flags are set. The ordering of the operands in both the fcmp and select instructions is important for the folding to occur. maxnum patterns:
The second pattern is supposed to make the order of the operands in the select instruction irrelevant. However, the pattern matching code uses the CmpInst::getInversePredicate method to invert the comparison predicate. This method doesn't take into account the fast-math flags, which can lead missing the folding opportunity. The patch extends the pattern matching code to handle unordered fcmp instructions. This allows the folding to occur even when the select instruction has the operands in the inverse order. New maxnum patterns:
The same changes are applied to the minnum intrinsic. Full diff: https://github.com/llvm/llvm-project/pull/112088.diff 5 Files Affected:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
index 358563a5fcd537..d3438764495efe 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -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))))
return replaceInstUsesWith(
SI, Builder.CreateBinaryIntrinsic(Intrinsic::maxnum, X, Y, &SI));
- if (match(&SI, m_OrdFMin(m_Value(X), m_Value(Y))))
+ if (match(&SI, m_OrdFMin(m_Value(X), m_Value(Y))) ||
+ match(&SI, m_UnordFMin(m_Value(X), m_Value(Y))))
return replaceInstUsesWith(
SI, Builder.CreateBinaryIntrinsic(Intrinsic::minnum, X, Y, &SI));
}
diff --git a/llvm/test/Transforms/InstCombine/clamp-to-minmax.ll b/llvm/test/Transforms/InstCombine/clamp-to-minmax.ll
index 1dd0b17e9f46dd..c6fee0914f0e78 100644
--- a/llvm/test/Transforms/InstCombine/clamp-to-minmax.ll
+++ b/llvm/test/Transforms/InstCombine/clamp-to-minmax.ll
@@ -67,10 +67,10 @@ define float @clamp_float_fast_ordered_nonstrict_minmax(float %x) {
; (X < C1) ? C1 : MIN(X, C2)
define float @clamp_float_fast_unordered_strict_maxmin(float %x) {
; CHECK-LABEL: @clamp_float_fast_unordered_strict_maxmin(
-; CHECK-NEXT: [[CMP2_INV:%.*]] = fcmp fast oge float [[X:%.*]], 2.550000e+02
-; CHECK-NEXT: [[MIN:%.*]] = select fast i1 [[CMP2_INV]], float 2.550000e+02, float [[X]]
-; CHECK-NEXT: [[R1:%.*]] = call fast float @llvm.maxnum.f32(float [[MIN]], float 1.000000e+00)
-; CHECK-NEXT: ret float [[R1]]
+; CHECK-NEXT: [[MIN:%.*]] = call fast float @llvm.minnum.f32(float [[X:%.*]], float 2.550000e+02)
+; CHECK-NEXT: [[CMP1:%.*]] = fcmp fast ult float [[X]], 1.000000e+00
+; CHECK-NEXT: [[R:%.*]] = select i1 [[CMP1]], float 1.000000e+00, float [[MIN]]
+; CHECK-NEXT: ret float [[R]]
;
%cmp2 = fcmp fast ult float %x, 255.0
%min = select i1 %cmp2, float %x, float 255.0
@@ -82,10 +82,10 @@ define float @clamp_float_fast_unordered_strict_maxmin(float %x) {
; (X <= C1) ? C1 : MIN(X, C2)
define float @clamp_float_fast_unordered_nonstrict_maxmin(float %x) {
; CHECK-LABEL: @clamp_float_fast_unordered_nonstrict_maxmin(
-; CHECK-NEXT: [[CMP2_INV:%.*]] = fcmp fast oge float [[X:%.*]], 2.550000e+02
-; CHECK-NEXT: [[MIN:%.*]] = select fast i1 [[CMP2_INV]], float 2.550000e+02, float [[X]]
-; CHECK-NEXT: [[R1:%.*]] = call fast float @llvm.maxnum.f32(float [[MIN]], float 1.000000e+00)
-; CHECK-NEXT: ret float [[R1]]
+; CHECK-NEXT: [[MIN:%.*]] = call fast float @llvm.minnum.f32(float [[X:%.*]], float 2.550000e+02)
+; CHECK-NEXT: [[CMP1:%.*]] = fcmp fast ule float [[X]], 1.000000e+00
+; CHECK-NEXT: [[R:%.*]] = select i1 [[CMP1]], float 1.000000e+00, float [[MIN]]
+; CHECK-NEXT: ret float [[R]]
;
%cmp2 = fcmp fast ult float %x, 255.0
%min = select i1 %cmp2, float %x, float 255.0
@@ -97,10 +97,10 @@ define float @clamp_float_fast_unordered_nonstrict_maxmin(float %x) {
; (X > C1) ? C1 : MAX(X, C2)
define float @clamp_float_fast_unordered_strict_minmax(float %x) {
; CHECK-LABEL: @clamp_float_fast_unordered_strict_minmax(
-; CHECK-NEXT: [[CMP2_INV:%.*]] = fcmp fast ole float [[X:%.*]], 1.000000e+00
-; CHECK-NEXT: [[MAX:%.*]] = select fast i1 [[CMP2_INV]], float 1.000000e+00, float [[X]]
-; CHECK-NEXT: [[R1:%.*]] = call fast float @llvm.minnum.f32(float [[MAX]], float 2.550000e+02)
-; CHECK-NEXT: ret float [[R1]]
+; CHECK-NEXT: [[MAX:%.*]] = call fast float @llvm.maxnum.f32(float [[X:%.*]], float 1.000000e+00)
+; CHECK-NEXT: [[CMP1:%.*]] = fcmp fast ugt float [[X]], 2.550000e+02
+; CHECK-NEXT: [[R:%.*]] = select i1 [[CMP1]], float 2.550000e+02, float [[MAX]]
+; CHECK-NEXT: ret float [[R]]
;
%cmp2 = fcmp fast ugt float %x, 1.0
%max = select i1 %cmp2, float %x, float 1.0
@@ -112,10 +112,10 @@ define float @clamp_float_fast_unordered_strict_minmax(float %x) {
; (X >= C1) ? C1 : MAX(X, C2)
define float @clamp_float_fast_unordered_nonstrict_minmax(float %x) {
; CHECK-LABEL: @clamp_float_fast_unordered_nonstrict_minmax(
-; CHECK-NEXT: [[CMP2_INV:%.*]] = fcmp fast ole float [[X:%.*]], 1.000000e+00
-; CHECK-NEXT: [[MAX:%.*]] = select fast i1 [[CMP2_INV]], float 1.000000e+00, float [[X]]
-; CHECK-NEXT: [[R1:%.*]] = call fast float @llvm.minnum.f32(float [[MAX]], float 2.550000e+02)
-; CHECK-NEXT: ret float [[R1]]
+; CHECK-NEXT: [[MAX:%.*]] = call fast float @llvm.maxnum.f32(float [[X:%.*]], float 1.000000e+00)
+; CHECK-NEXT: [[CMP1:%.*]] = fcmp fast uge float [[X]], 2.550000e+02
+; CHECK-NEXT: [[R:%.*]] = select i1 [[CMP1]], float 2.550000e+02, float [[MAX]]
+; CHECK-NEXT: ret float [[R]]
;
%cmp2 = fcmp fast ugt float %x, 1.0
%max = select i1 %cmp2, float %x, float 1.0
@@ -127,13 +127,12 @@ define float @clamp_float_fast_unordered_nonstrict_minmax(float %x) {
; Some more checks with fast
; (X > 1.0) ? min(x, 255.0) : 1.0
-; That did not match because select was in inverse order.
define float @clamp_test_1(float %x) {
; CHECK-LABEL: @clamp_test_1(
-; CHECK-NEXT: [[INNER_CMP_INV:%.*]] = fcmp fast oge float [[X:%.*]], 2.550000e+02
-; CHECK-NEXT: [[INNER_SEL:%.*]] = select fast i1 [[INNER_CMP_INV]], float 2.550000e+02, float [[X]]
-; CHECK-NEXT: [[R1:%.*]] = call fast float @llvm.maxnum.f32(float [[INNER_SEL]], float 1.000000e+00)
-; CHECK-NEXT: ret float [[R1]]
+; CHECK-NEXT: [[INNER_SEL:%.*]] = call fast float @llvm.minnum.f32(float [[X:%.*]], float 2.550000e+02)
+; CHECK-NEXT: [[OUTER_CMP:%.*]] = fcmp fast ugt float [[X]], 1.000000e+00
+; CHECK-NEXT: [[R:%.*]] = select i1 [[OUTER_CMP]], float [[INNER_SEL]], float 1.000000e+00
+; CHECK-NEXT: ret float [[R]]
;
%inner_cmp = fcmp fast ult float %x, 255.0
%inner_sel = select i1 %inner_cmp, float %x, float 255.0
@@ -147,8 +146,7 @@ define float @clamp_test_1(float %x) {
; Like @clamp_test_1 but HighConst < LowConst
define float @clamp_negative_wrong_const(float %x) {
; CHECK-LABEL: @clamp_negative_wrong_const(
-; CHECK-NEXT: [[INNER_CMP_INV:%.*]] = fcmp fast oge float [[X:%.*]], 2.550000e+02
-; CHECK-NEXT: [[INNER_SEL:%.*]] = select fast i1 [[INNER_CMP_INV]], float 2.550000e+02, float [[X]]
+; CHECK-NEXT: [[INNER_SEL:%.*]] = call fast float @llvm.minnum.f32(float [[X:%.*]], float 2.550000e+02)
; CHECK-NEXT: [[OUTER_CMP:%.*]] = fcmp fast ugt float [[X]], 5.120000e+02
; CHECK-NEXT: [[R:%.*]] = select i1 [[OUTER_CMP]], float [[INNER_SEL]], float 5.120000e+02
; CHECK-NEXT: ret float [[R]]
@@ -163,8 +161,7 @@ define float @clamp_negative_wrong_const(float %x) {
; Like @clamp_test_1 but both are min
define float @clamp_negative_same_op(float %x) {
; CHECK-LABEL: @clamp_negative_same_op(
-; CHECK-NEXT: [[INNER_CMP_INV:%.*]] = fcmp fast oge float [[X:%.*]], 2.550000e+02
-; CHECK-NEXT: [[INNER_SEL:%.*]] = select fast i1 [[INNER_CMP_INV]], float 2.550000e+02, float [[X]]
+; CHECK-NEXT: [[INNER_SEL:%.*]] = call fast float @llvm.minnum.f32(float [[X:%.*]], float 2.550000e+02)
; CHECK-NEXT: [[OUTER_CMP:%.*]] = fcmp fast ult float [[X]], 1.000000e+00
; CHECK-NEXT: [[R:%.*]] = select i1 [[OUTER_CMP]], float [[INNER_SEL]], float 1.000000e+00
; CHECK-NEXT: ret float [[R]]
diff --git a/llvm/test/Transforms/InstCombine/minmax-fold.ll b/llvm/test/Transforms/InstCombine/minmax-fold.ll
index 26cd4996e687d5..ec1c7aff409661 100644
--- a/llvm/test/Transforms/InstCombine/minmax-fold.ll
+++ b/llvm/test/Transforms/InstCombine/minmax-fold.ll
@@ -852,10 +852,8 @@ define i32 @common_factor_umax_extra_use_both(i32 %a, i32 %b, i32 %c) {
define float @not_min_of_min(i8 %i, float %x) {
; CHECK-LABEL: @not_min_of_min(
-; CHECK-NEXT: [[CMP1_INV:%.*]] = fcmp fast oge float [[X:%.*]], 1.000000e+00
-; CHECK-NEXT: [[MIN1:%.*]] = select fast i1 [[CMP1_INV]], float 1.000000e+00, float [[X]]
-; CHECK-NEXT: [[CMP2_INV:%.*]] = fcmp fast oge float [[X]], 2.000000e+00
-; CHECK-NEXT: [[MIN2:%.*]] = select fast i1 [[CMP2_INV]], float 2.000000e+00, float [[X]]
+; CHECK-NEXT: [[MIN1:%.*]] = call fast float @llvm.minnum.f32(float [[X:%.*]], float 1.000000e+00)
+; CHECK-NEXT: [[MIN2:%.*]] = call fast float @llvm.minnum.f32(float [[X]], float 2.000000e+00)
; CHECK-NEXT: [[CMP3:%.*]] = icmp ult i8 [[I:%.*]], 16
; CHECK-NEXT: [[R:%.*]] = select i1 [[CMP3]], float [[MIN1]], float [[MIN2]]
; CHECK-NEXT: ret float [[R]]
diff --git a/llvm/test/Transforms/InstCombine/minmax-fp.ll b/llvm/test/Transforms/InstCombine/minmax-fp.ll
index b9e46caa63753a..1276b7b3e3867d 100644
--- a/llvm/test/Transforms/InstCombine/minmax-fp.ll
+++ b/llvm/test/Transforms/InstCombine/minmax-fp.ll
@@ -160,8 +160,7 @@ define i8 @t9(float %a) {
; Either operand could be NaN, but fast modifier applied.
define i8 @t11(float %a, float %b) {
; CHECK-LABEL: @t11(
-; CHECK-NEXT: [[DOTINV:%.*]] = fcmp fast oge float [[B:%.*]], [[A:%.*]]
-; CHECK-NEXT: [[DOTV:%.*]] = select fast i1 [[DOTINV]], float [[A]], float [[B]]
+; CHECK-NEXT: [[DOTV:%.*]] = call fast float @llvm.minnum.f32(float [[B:%.*]], float [[A:%.*]])
; CHECK-NEXT: [[TMP1:%.*]] = fptosi float [[DOTV]] to i8
; CHECK-NEXT: ret i8 [[TMP1]]
;
@@ -282,8 +281,7 @@ define float @fneg_fmax(float %x, float %y) {
define <2 x float> @fsub_fmax(<2 x float> %x, <2 x float> %y) {
; CHECK-LABEL: @fsub_fmax(
-; CHECK-NEXT: [[COND_INV:%.*]] = fcmp nnan nsz ogt <2 x float> [[X:%.*]], [[Y:%.*]]
-; CHECK-NEXT: [[MAX_V:%.*]] = select nnan nsz <2 x i1> [[COND_INV]], <2 x float> [[Y]], <2 x float> [[X]]
+; CHECK-NEXT: [[MAX_V:%.*]] = call nnan nsz <2 x float> @llvm.minnum.v2f32(<2 x float> [[X:%.*]], <2 x float> [[Y:%.*]])
; CHECK-NEXT: [[MAX:%.*]] = fneg <2 x float> [[MAX_V]]
; CHECK-NEXT: ret <2 x float> [[MAX]]
;
@@ -310,8 +308,7 @@ define <2 x double> @fsub_fmin(<2 x double> %x, <2 x double> %y) {
define double @fneg_fmin(double %x, double %y) {
; CHECK-LABEL: @fneg_fmin(
-; CHECK-NEXT: [[COND_INV:%.*]] = fcmp nnan nsz olt double [[X:%.*]], [[Y:%.*]]
-; CHECK-NEXT: [[MAX_V:%.*]] = select nnan nsz i1 [[COND_INV]], double [[Y]], double [[X]]
+; CHECK-NEXT: [[MAX_V:%.*]] = call nnan nsz double @llvm.maxnum.f64(double [[X:%.*]], double [[Y:%.*]])
; CHECK-NEXT: [[MAX:%.*]] = fneg double [[MAX_V]]
; CHECK-NEXT: ret double [[MAX]]
;
diff --git a/llvm/test/Transforms/InstCombine/unordered-fcmp-select.ll b/llvm/test/Transforms/InstCombine/unordered-fcmp-select.ll
index 62c12c15a075cd..b164dd983a8925 100644
--- a/llvm/test/Transforms/InstCombine/unordered-fcmp-select.ll
+++ b/llvm/test/Transforms/InstCombine/unordered-fcmp-select.ll
@@ -115,7 +115,7 @@ define float @select_max_ugt_2_use_cmp(float %a, float %b) {
; CHECK-LABEL: @select_max_ugt_2_use_cmp(
; CHECK-NEXT: [[CMP:%.*]] = fcmp reassoc ugt float [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: call void @foo(i1 [[CMP]])
-; CHECK-NEXT: [[SEL:%.*]] = select fast i1 [[CMP]], float [[A]], float [[B]]
+; CHECK-NEXT: [[SEL:%.*]] = call fast float @llvm.maxnum.f32(float [[A]], float [[B]])
; CHECK-NEXT: ret float [[SEL]]
;
%cmp = fcmp reassoc ugt float %a, %b
|
arsenm
added
the
llvm:instcombine
arsenm
reviewed
Oct 14, 2024
Comment on lines
3837
to
3838
| if (match(&SI, m_OrdFMax(m_Value(X), m_Value(Y))) || | ||
| match(&SI, m_UnordFMax(m_Value(X), m_Value(Y)))) |
There was a problem hiding this comment.
It's a bit clumsy to have to match the two. If we're going to have dedicated matchers for it, is it worth having an OrdOrUnord variant?
There was a problem hiding this comment.
Okay. I'll refactor this code.
Should I update other uses of m_OrdFMax and m_UnordFMax uses in this patch?
There was a problem hiding this comment.
It's not that bad, can do in a follow up if it's worth it. I haven't looked at the other users of these, maybe they are missing this case too?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Added new matchers in 1713edf.
| ; CHECK-NEXT: [[INNER_SEL:%.*]] = select fast i1 [[INNER_CMP_INV]], float 2.550000e+02, float [[X]] | ||
| ; CHECK-NEXT: [[R1:%.*]] = call fast float @llvm.maxnum.f32(float [[INNER_SEL]], float 1.000000e+00) | ||
| ; CHECK-NEXT: ret float [[R1]] | ||
| ; CHECK-NEXT: [[INNER_SEL:%.*]] = call fast float @llvm.minnum.f32(float [[X:%.*]], float 2.550000e+02) |
There was a problem hiding this comment.
This traded a maxnum at the end for a minnum at the beginning. But I don't see how either of these matched. The fast flags are on the fcmp, and not the select?
I suppose the nsz is implied on the select by the usage, but it isn't propagated?
There was a problem hiding this comment.
It happens due to this transformation: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp#L3773-L3794. The first pass of InstCombine propagates fcmp fast math flags to the select instruction.
Considering that all operands of select are coming from fcmp, the propagation is valid. In general, it should be the union of fast math flags of both instructions.
llvmbot
added
llvm:ir
llvm:analysis
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
arsenm
approved these changes
Oct 15, 2024
|
@arsenm, thanks a lot for the review. Would you mind merging the pull request, please? I lost write access. |
DanielCChen
pushed a commit
to DanielCChen/llvm-project
that referenced
this pull request
Oct 16, 2024
…lvm#112088) Today, InstCombine can fold fcmp+select patterns to minnum/maxnum intrinsics when the nnan and nsz flags are set. The ordering of the operands in both the fcmp and select instructions is important for the folding to occur. maxnum patterns: 1. (a op b) ? a : b -> maxnum(a, b), where op is one of {ogt, oge} 2. (a op b) ? b : a -> maxnum(a, b), where op is one of {ule, ult} The second pattern is supposed to make the order of the operands in the select instruction irrelevant. However, the pattern matching code uses the CmpInst::getInversePredicate method to invert the comparison predicate. This method doesn't take into account the fast-math flags, which can lead missing the folding opportunity. The patch extends the pattern matching code to handle unordered fcmp instructions. This allows the folding to occur even when the select instruction has the operands in the inverse order. New maxnum patterns: 1. (a op b) ? a : b -> maxnum(a, b), where op is one of {ugt, uge} 2. (a op b) ? b : a -> maxnum(a, b), where op is one of {ole, olt} The same changes are applied to the minnum intrinsic.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Today, InstCombine can fold fcmp+select patterns to minnum/maxnum intrinsics when the nnan and nsz flags are set. The ordering of the operands in both the fcmp and select instructions is important for the folding to occur.
maxnum patterns:
The second pattern is supposed to make the order of the operands in the select instruction irrelevant. However, the pattern matching code uses the CmpInst::getInversePredicate method to invert the comparison predicate. This method doesn't take into account the fast-math flags, which can lead missing the folding opportunity.
The patch extends the pattern matching code to handle unordered fcmp instructions. This allows the folding to occur even when the select instruction has the operands in the inverse order.
New maxnum patterns:
The same changes are applied to the minnum intrinsic.