Skip to content

[InstCombine] Fix for folding select into floating point binary operators. #83200

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 1 commit into from
Mar 19, 2024
Merged

[InstCombine] Fix for folding select into floating point binary operators. #83200

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 23 additions & 13 deletions llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,19 +536,29 @@ Instruction *InstCombinerImpl::foldSelectIntoOp(SelectInst &SI, Value *TrueVal,
// between 0, 1 and -1.
const APInt *OOpC;
bool OOpIsAPInt = match(OOp, m_APInt(OOpC));
if (!isa<Constant>(OOp) ||
(OOpIsAPInt && isSelect01(C->getUniqueInteger(), *OOpC))) {
Value *NewSel = Builder.CreateSelect(SI.getCondition(), Swapped ? C : OOp,
Swapped ? OOp : C, "", &SI);
if (isa<FPMathOperator>(&SI))
cast<Instruction>(NewSel)->setFastMathFlags(FMF);
NewSel->takeName(TVI);
BinaryOperator *BO =
BinaryOperator::Create(TVI->getOpcode(), FalseVal, NewSel);
BO->copyIRFlags(TVI);
return BO;
}
return nullptr;
if (isa<Constant>(OOp) &&
(!OOpIsAPInt || !isSelect01(C->getUniqueInteger(), *OOpC)))
return nullptr;

// If the false value is a NaN then we have that the floating point math
// operation in the transformed code may not preserve the exact NaN
// bit-pattern -- e.g. `fadd sNaN, 0.0 -> qNaN`.
// This makes the transformation incorrect since the original program would
// have preserved the exact NaN bit-pattern.
// Avoid the folding if the false value might be a NaN.
if (isa<FPMathOperator>(&SI) &&
!computeKnownFPClass(FalseVal, FMF, fcNan, &SI).isKnownNeverNaN())
Copy link
Contributor

Choose a reason for hiding this comment

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

This ought to try to make use of the nnan flag on the select before trying the computeKnownFPClass query. There's a wrapper around computeKnownFPClass which takes an additional FastMathFlags

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm sorry, I'm sometimes lost here.
There's a wrapper around computeKnownFPClass which takes an additional FastMathFlags
Did you mean
The wrapper around computeKnownFPClass called here takes an additional FastMathFlags? That's how I see here, InstCombineSelect.cpp defines two wrappers around computeKnownFPClass:

  KnownFPClass computeKnownFPClass(Value *Val, FastMathFlags FMF,
                                   FPClassTest Interested = fcAllFlags,
                                   const Instruction *CtxI = nullptr,
                                   unsigned Depth = 0) const

and

  KnownFPClass computeKnownFPClass(Value *Val,
                                   FPClassTest Interested = fcAllFlags,
                                   const Instruction *CtxI = nullptr,
                                   unsigned Depth = 0) const

The one called here is the one which takes FastMathFlags FMF.

Copy link
Contributor

Choose a reason for hiding this comment

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

This ought to try to make use of the nnan flag on the select before trying the computeKnownFPClass query

Could you say what use would that be?

return nullptr;

Value *NewSel = Builder.CreateSelect(SI.getCondition(), Swapped ? C : OOp,
Swapped ? OOp : C, "", &SI);
if (isa<FPMathOperator>(&SI))
cast<Instruction>(NewSel)->setFastMathFlags(FMF);
NewSel->takeName(TVI);
BinaryOperator *BO =
BinaryOperator::Create(TVI->getOpcode(), FalseVal, NewSel);
BO->copyIRFlags(TVI);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can or in the value flags from the select

Copy link
Contributor

Choose a reason for hiding this comment

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

alive2 is really slow here without known-no-infs on the inputs though, so I'm not sure the exact flag rewrite rules yet

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe (nnan_bo | nnan_select) | (others_flags_bo & other_flags_select)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can or in the value flags from the select

I don't even know what does it mean :(

Copy link
Contributor

@pawosm-arm pawosm-arm Oct 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think this can or in the value flags from the select

I don't even know what does it mean :(

Ok, I guess I've deciphered it. I guess what you propose is to 'or' the flags copied from TVI with value flags from the select. Trouble is, SI don't have any FMF flags, they were not generated by simplifyCFG pass since they were lost in SROA pass.

return BO;
};

if (Instruction *R = TryFoldSelectIntoOp(SI, TrueVal, FalseVal, false))
Expand Down
56 changes: 28 additions & 28 deletions llvm/test/Transforms/InstCombine/fold-select-fmul-if-zero.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,28 +427,28 @@ define float @fmul_by_snan_if_0_oeq_zero_f32(float %x) {
define float @fmul_by_var_if_0_oeq_zero_f32(float %x, float %y) {
; CHECK-LABEL: @fmul_by_var_if_0_oeq_zero_f32(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul float %x, %y
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

define float @fmul_by_fabs_var_if_0_oeq_zero_f32(float %x, float %y) {
; CHECK-LABEL: @fmul_by_fabs_var_if_0_oeq_zero_f32(
; CHECK-NEXT: [[Y_FABS:%.*]] = call float @llvm.fabs.f32(float [[Y:%.*]])
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[Y_FABS]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[Y_FABS]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%y.fabs = call float @llvm.fabs.f32(float %y)
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul float %x, %y.fabs
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

Expand All @@ -467,55 +467,55 @@ define float @fmul_by_fabs_nnan_ninf_var_if_0_oeq_zero_f32(float %x, float %y) {
define float @fmul_by_var_if_0_oeq_zero_f32_nsz_fmul(float %x, float %y) {
; CHECK-LABEL: @fmul_by_var_if_0_oeq_zero_f32_nsz_fmul(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul nsz float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul nsz float %x, %y
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

; nsz ninf is not sufficient
define float @fmul_by_var_if_0_oeq_zero_f32_nsz_ninf_fmul(float %x, float %y) {
; CHECK-LABEL: @fmul_by_var_if_0_oeq_zero_f32_nsz_ninf_fmul(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul ninf nsz float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul nsz ninf float %x, %y
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

; nsz nnan is not sufficient
define float @fmul_by_var_if_0_oeq_zero_f32_nsz_nnan_fmul(float %x, float %y) {
; CHECK-LABEL: @fmul_by_var_if_0_oeq_zero_f32_nsz_nnan_fmul(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul nnan nsz float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul nsz nnan float %x, %y
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

; nnan ninf is not sufficient
define float @fmul_by_var_if_0_oeq_zero_f32_nnan_ninf_fmul(float %x, float %y) {
; CHECK-LABEL: @fmul_by_var_if_0_oeq_zero_f32_nnan_ninf_fmul(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul nnan ninf float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul nnan ninf float %x, %y
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

Expand Down Expand Up @@ -558,53 +558,53 @@ define float @fmul_by_var_if_0_oeq_zero_f32_fmul_nnan_ninf_select_nsz_inverted(f
define float @fmul_by_var_if_0_oeq_zero_f32_fmul_nnan_ninf_nsz(float %x, float %y) {
; CHECK-LABEL: @fmul_by_var_if_0_oeq_zero_f32_fmul_nnan_ninf_nsz(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul nnan ninf nsz float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul nnan ninf nsz float %x, %y
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

define float @fmul_by_var_if_0_oeq_zero_f32_fmul_nnan_ninf_nsz_commuted(float %x, float %y) {
; CHECK-LABEL: @fmul_by_var_if_0_oeq_zero_f32_fmul_nnan_ninf_nsz_commuted(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul nnan ninf nsz float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul nnan ninf nsz float %y, %x
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

; test computeKnownFPClass is checked
define float @fmul_by_var_if_0_oeq_zero_f32_fmul_nnan_ninf_select_known_never_negzero(float %x, float nofpclass(nzero) %y) {
; CHECK-LABEL: @fmul_by_var_if_0_oeq_zero_f32_fmul_nnan_ninf_select_known_never_negzero(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul nnan ninf float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul nnan ninf float %x, %y
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

define float @fmul_by_var_if_0_oeq_zero_f32_fmul_nnan_ninf_select_known_never_negzero_negsub(float %x, float nofpclass(nzero nsub) %y) {
; CHECK-LABEL: @fmul_by_var_if_0_oeq_zero_f32_fmul_nnan_ninf_select_known_never_negzero_negsub(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul nnan ninf float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul nnan ninf float %x, %y
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

Expand All @@ -622,26 +622,26 @@ define float @fmul_by_var_if_0_oeq_zero_f32_known_never_nan_inf_select_nsz(float
define float @fmul_by_var_if_0_oeq_zero_f32_fmul_known_never_nan_inf_negzero(float %x, float nofpclass(nan inf nzero) %y) {
; CHECK-LABEL: @fmul_by_var_if_0_oeq_zero_f32_fmul_known_never_nan_inf_negzero(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul float %x, %y
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

define float @fmul_by_var_if_0_oeq_zero_f32_fmul_known_never_nan_inf_negzero_nsub(float %x, float nofpclass(nan inf nzero nsub) %y) {
; CHECK-LABEL: @fmul_by_var_if_0_oeq_zero_f32_fmul_known_never_nan_inf_negzero_nsub(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[Y:%.*]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul float %x, %y
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

Expand Down Expand Up @@ -692,26 +692,26 @@ define float @fmul_by_var_if_not_one_0_zero_f32_assume_finite_fmul_nsz(float %x,
define float @fmul_by_self_if_0_oeq_zero_f32(float %x) {
; CHECK-LABEL: @fmul_by_self_if_0_oeq_zero_f32(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[X]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[X]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul float %x, %x
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

define float @fmul_by_self_if_0_oeq_zero_f32_fmul_nnan_ninf_nsz(float %x) {
; CHECK-LABEL: @fmul_by_self_if_0_oeq_zero_f32_fmul_nnan_ninf_nsz(
; CHECK-NEXT: [[X_IS_ZERO:%.*]] = fcmp oeq float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select i1 [[X_IS_ZERO]], float [[X]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_X:%.*]] = select nnan i1 [[X_IS_ZERO]], float [[X]], float 1.000000e+00
; CHECK-NEXT: [[SCALED_IF_DENORMAL:%.*]] = fmul nnan ninf nsz float [[SCALED_X]], [[X]]
; CHECK-NEXT: ret float [[SCALED_IF_DENORMAL]]
;
%x.is.zero = fcmp oeq float %x, 0.0
%scaled.x = fmul nnan ninf nsz float %x, %x
%scaled.if.denormal = select i1 %x.is.zero, float %scaled.x, float %x
%scaled.if.denormal = select nnan i1 %x.is.zero, float %scaled.x, float %x
ret float %scaled.if.denormal
}

Expand Down
Loading