Skip to content

[InstCombine] Extend fcmp+select folding to minnum/maxnum intrinsics #112088

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Oct 15, 2024
Merged

[InstCombine] Extend fcmp+select folding to minnum/maxnum intrinsics #112088

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76581cb
[InstCombine] Extend fcmp+select folding to minnum/maxnum intrinsics
bader Oct 12, 2024
1713edf
Add m_OrdOrUnordFMax and m_OrdOrUnordFMin matchers
bader Oct 14, 2024
a821fe6
Apply clang-format suggestions.
bader Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))))
Copy link
Contributor

@arsenm arsenm Oct 14, 2024

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@bader bader Oct 14, 2024

Choose a reason for hiding this comment

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

Okay. I'll refactor this code.
Should I update other uses of m_OrdFMax and m_UnordFMax uses in this patch?

Copy link
Contributor

@arsenm arsenm Oct 14, 2024

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@bader bader Oct 14, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@bader bader Oct 14, 2024

Choose a reason for hiding this comment

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

Added new matchers in 1713edf.

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));
}
Expand Down
47 changes: 22 additions & 25 deletions llvm/test/Transforms/InstCombine/clamp-to-minmax.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Copy link
Contributor

@arsenm arsenm Oct 14, 2024

Choose a reason for hiding this comment

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

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?

Copy link
Contributor Author

@bader bader Oct 14, 2024

Choose a reason for hiding this comment

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

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.

; 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
Expand All @@ -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]]
Expand All @@ -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]]
Expand Down
6 changes: 2 additions & 4 deletions llvm/test/Transforms/InstCombine/minmax-fold.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
Expand Down
9 changes: 3 additions & 6 deletions llvm/test/Transforms/InstCombine/minmax-fp.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]]
;
Expand Down Expand Up @@ -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]]
;
Expand All @@ -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]]
;
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/Transforms/InstCombine/unordered-fcmp-select.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading