Skip to content

[InstCombine] Fix Failure to convert vector fp comparisons that can be represented as integers #82241 #83274

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 2 commits into from
Mar 5, 2024
Merged

[InstCombine] Fix Failure to convert vector fp comparisons that can be represented as integers #82241 #83274

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
07773ad
[InstCombine] Fix Failure to convert vector fp comparisons that can b…
Feb 28, 2024
b24e99d
Refactored code
Feb 29, 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
92 changes: 47 additions & 45 deletions llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7286,8 +7286,9 @@ Instruction *InstCombinerImpl::visitICmpInst(ICmpInst &I) {
Instruction *InstCombinerImpl::foldFCmpIntToFPConst(FCmpInst &I,
Instruction *LHSI,
Constant *RHSC) {
if (!isa<ConstantFP>(RHSC)) return nullptr;
const APFloat &RHS = cast<ConstantFP>(RHSC)->getValueAPF();
const APFloat *RHS;
if (!match(RHSC, m_APFloat(RHS)))
return nullptr;

// Get the width of the mantissa. We don't want to hack on conversions that
// might lose information from the integer, e.g. "i64 -> float"
Expand All @@ -7302,20 +7303,20 @@ Instruction *InstCombinerImpl::foldFCmpIntToFPConst(FCmpInst &I,
FCmpInst::Predicate P = I.getPredicate();
bool IsExact = false;
APSInt RHSCvt(IntWidth, LHSUnsigned);
RHS.convertToInteger(RHSCvt, APFloat::rmNearestTiesToEven, &IsExact);
RHS->convertToInteger(RHSCvt, APFloat::rmNearestTiesToEven, &IsExact);

// If the floating point constant isn't an integer value, we know if we will
// ever compare equal / not equal to it.
if (!IsExact) {
// TODO: Can never be -0.0 and other non-representable values
APFloat RHSRoundInt(RHS);
APFloat RHSRoundInt(*RHS);
RHSRoundInt.roundToIntegral(APFloat::rmNearestTiesToEven);
if (RHS != RHSRoundInt) {
if (*RHS != RHSRoundInt) {
if (P == FCmpInst::FCMP_OEQ || P == FCmpInst::FCMP_UEQ)
return replaceInstUsesWith(I, Builder.getFalse());
return replaceInstUsesWith(I, ConstantInt::getFalse(I.getType()));

assert(P == FCmpInst::FCMP_ONE || P == FCmpInst::FCMP_UNE);
return replaceInstUsesWith(I, Builder.getTrue());
return replaceInstUsesWith(I, ConstantInt::getTrue(I.getType()));
}
}

Expand All @@ -7332,9 +7333,9 @@ Instruction *InstCombinerImpl::foldFCmpIntToFPConst(FCmpInst &I,
// to distinguish it from one less than that value.
if ((int)IntWidth > MantissaWidth) {
// Conversion would lose accuracy. Check if loss can impact comparison.
int Exp = ilogb(RHS);
int Exp = ilogb(*RHS);
if (Exp == APFloat::IEK_Inf) {
int MaxExponent = ilogb(APFloat::getLargest(RHS.getSemantics()));
int MaxExponent = ilogb(APFloat::getLargest(RHS->getSemantics()));
if (MaxExponent < (int)IntWidth - !LHSUnsigned)
// Conversion could create infinity.
return nullptr;
Expand All @@ -7350,7 +7351,7 @@ Instruction *InstCombinerImpl::foldFCmpIntToFPConst(FCmpInst &I,
// Otherwise, we can potentially simplify the comparison. We know that it
// will always come through as an integer value and we know the constant is
// not a NAN (it would have been previously simplified).
assert(!RHS.isNaN() && "NaN comparison not already folded!");
assert(!RHS->isNaN() && "NaN comparison not already folded!");

ICmpInst::Predicate Pred;
switch (I.getPredicate()) {
Expand Down Expand Up @@ -7380,9 +7381,9 @@ Instruction *InstCombinerImpl::foldFCmpIntToFPConst(FCmpInst &I,
Pred = ICmpInst::ICMP_NE;
break;
case FCmpInst::FCMP_ORD:
return replaceInstUsesWith(I, Builder.getTrue());
return replaceInstUsesWith(I, ConstantInt::getTrue(I.getType()));
case FCmpInst::FCMP_UNO:
return replaceInstUsesWith(I, Builder.getFalse());
return replaceInstUsesWith(I, ConstantInt::getFalse(I.getType()));
}

// Now we know that the APFloat is a normal number, zero or inf.
Expand All @@ -7392,50 +7393,50 @@ Instruction *InstCombinerImpl::foldFCmpIntToFPConst(FCmpInst &I,
if (!LHSUnsigned) {
// If the RHS value is > SignedMax, fold the comparison. This handles +INF
// and large values.
APFloat SMax(RHS.getSemantics());
APFloat SMax(RHS->getSemantics());
SMax.convertFromAPInt(APInt::getSignedMaxValue(IntWidth), true,
APFloat::rmNearestTiesToEven);
if (SMax < RHS) { // smax < 13123.0
if (SMax < *RHS) { // smax < 13123.0
if (Pred == ICmpInst::ICMP_NE || Pred == ICmpInst::ICMP_SLT ||
Pred == ICmpInst::ICMP_SLE)
return replaceInstUsesWith(I, Builder.getTrue());
return replaceInstUsesWith(I, Builder.getFalse());
return replaceInstUsesWith(I, ConstantInt::getTrue(I.getType()));
return replaceInstUsesWith(I, ConstantInt::getFalse(I.getType()));
}
} else {
// If the RHS value is > UnsignedMax, fold the comparison. This handles
// +INF and large values.
APFloat UMax(RHS.getSemantics());
APFloat UMax(RHS->getSemantics());
UMax.convertFromAPInt(APInt::getMaxValue(IntWidth), false,
APFloat::rmNearestTiesToEven);
if (UMax < RHS) { // umax < 13123.0
if (UMax < *RHS) { // umax < 13123.0
if (Pred == ICmpInst::ICMP_NE || Pred == ICmpInst::ICMP_ULT ||
Pred == ICmpInst::ICMP_ULE)
return replaceInstUsesWith(I, Builder.getTrue());
return replaceInstUsesWith(I, Builder.getFalse());
return replaceInstUsesWith(I, ConstantInt::getTrue(I.getType()));
return replaceInstUsesWith(I, ConstantInt::getFalse(I.getType()));
}
}

if (!LHSUnsigned) {
// See if the RHS value is < SignedMin.
APFloat SMin(RHS.getSemantics());
APFloat SMin(RHS->getSemantics());
SMin.convertFromAPInt(APInt::getSignedMinValue(IntWidth), true,
APFloat::rmNearestTiesToEven);
if (SMin > RHS) { // smin > 12312.0
if (SMin > *RHS) { // smin > 12312.0
if (Pred == ICmpInst::ICMP_NE || Pred == ICmpInst::ICMP_SGT ||
Pred == ICmpInst::ICMP_SGE)
return replaceInstUsesWith(I, Builder.getTrue());
return replaceInstUsesWith(I, Builder.getFalse());
return replaceInstUsesWith(I, ConstantInt::getTrue(I.getType()));
return replaceInstUsesWith(I, ConstantInt::getFalse(I.getType()));
}
} else {
// See if the RHS value is < UnsignedMin.
APFloat UMin(RHS.getSemantics());
APFloat UMin(RHS->getSemantics());
UMin.convertFromAPInt(APInt::getMinValue(IntWidth), false,
APFloat::rmNearestTiesToEven);
if (UMin > RHS) { // umin > 12312.0
if (UMin > *RHS) { // umin > 12312.0
if (Pred == ICmpInst::ICMP_NE || Pred == ICmpInst::ICMP_UGT ||
Pred == ICmpInst::ICMP_UGE)
return replaceInstUsesWith(I, Builder.getTrue());
return replaceInstUsesWith(I, Builder.getFalse());
return replaceInstUsesWith(I, ConstantInt::getTrue(I.getType()));
return replaceInstUsesWith(I, ConstantInt::getFalse(I.getType()));
}
}

Expand All @@ -7445,66 +7446,66 @@ Instruction *InstCombinerImpl::foldFCmpIntToFPConst(FCmpInst &I,
// Don't do this for zero, because -0.0 is not fractional.
APSInt RHSInt(IntWidth, LHSUnsigned);
bool IsExact;
RHS.convertToInteger(RHSInt, APFloat::rmTowardZero, &IsExact);
if (!RHS.isZero()) {
RHS->convertToInteger(RHSInt, APFloat::rmTowardZero, &IsExact);
if (!RHS->isZero()) {
if (!IsExact) {
// If we had a comparison against a fractional value, we have to adjust
// the compare predicate and sometimes the value. RHSC is rounded towards
// zero at this point.
switch (Pred) {
default: llvm_unreachable("Unexpected integer comparison!");
case ICmpInst::ICMP_NE: // (float)int != 4.4 --> true
return replaceInstUsesWith(I, Builder.getTrue());
return replaceInstUsesWith(I, ConstantInt::getTrue(I.getType()));
case ICmpInst::ICMP_EQ: // (float)int == 4.4 --> false
return replaceInstUsesWith(I, Builder.getFalse());
return replaceInstUsesWith(I, ConstantInt::getFalse(I.getType()));
case ICmpInst::ICMP_ULE:
// (float)int <= 4.4 --> int <= 4
// (float)int <= -4.4 --> false
if (RHS.isNegative())
return replaceInstUsesWith(I, Builder.getFalse());
if (RHS->isNegative())
return replaceInstUsesWith(I, ConstantInt::getFalse(I.getType()));
break;
case ICmpInst::ICMP_SLE:
// (float)int <= 4.4 --> int <= 4
// (float)int <= -4.4 --> int < -4
if (RHS.isNegative())
if (RHS->isNegative())
Pred = ICmpInst::ICMP_SLT;
break;
case ICmpInst::ICMP_ULT:
// (float)int < -4.4 --> false
// (float)int < 4.4 --> int <= 4
if (RHS.isNegative())
return replaceInstUsesWith(I, Builder.getFalse());
if (RHS->isNegative())
return replaceInstUsesWith(I, ConstantInt::getFalse(I.getType()));
Pred = ICmpInst::ICMP_ULE;
break;
case ICmpInst::ICMP_SLT:
// (float)int < -4.4 --> int < -4
// (float)int < 4.4 --> int <= 4
if (!RHS.isNegative())
if (!RHS->isNegative())
Pred = ICmpInst::ICMP_SLE;
break;
case ICmpInst::ICMP_UGT:
// (float)int > 4.4 --> int > 4
// (float)int > -4.4 --> true
if (RHS.isNegative())
return replaceInstUsesWith(I, Builder.getTrue());
if (RHS->isNegative())
return replaceInstUsesWith(I, ConstantInt::getTrue(I.getType()));
break;
case ICmpInst::ICMP_SGT:
// (float)int > 4.4 --> int > 4
// (float)int > -4.4 --> int >= -4
if (RHS.isNegative())
if (RHS->isNegative())
Pred = ICmpInst::ICMP_SGE;
break;
case ICmpInst::ICMP_UGE:
// (float)int >= -4.4 --> true
// (float)int >= 4.4 --> int > 4
if (RHS.isNegative())
return replaceInstUsesWith(I, Builder.getTrue());
if (RHS->isNegative())
return replaceInstUsesWith(I, ConstantInt::getTrue(I.getType()));
Pred = ICmpInst::ICMP_UGT;
break;
case ICmpInst::ICMP_SGE:
// (float)int >= -4.4 --> int >= -4
// (float)int >= 4.4 --> int > 4
if (!RHS.isNegative())
if (!RHS->isNegative())
Pred = ICmpInst::ICMP_SGT;
break;
}
Expand All @@ -7513,7 +7514,8 @@ Instruction *InstCombinerImpl::foldFCmpIntToFPConst(FCmpInst &I,

// Lower this FP comparison into an appropriate integer version of the
// comparison.
return new ICmpInst(Pred, LHSI->getOperand(0), Builder.getInt(RHSInt));
return new ICmpInst(Pred, LHSI->getOperand(0),
ConstantInt::get(LHSI->getOperand(0)->getType(), RHSInt));
}

/// Fold (C / X) < 0.0 --> X < 0.0 if possible. Swap predicate if necessary.
Expand Down
58 changes: 58 additions & 0 deletions llvm/test/Transforms/InstCombine/cast-int-fcmp-eq-0.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,3 +509,61 @@ define i1 @i128_cast_cmp_oeq_int_inf_uitofp(i128 %i) {
%cmp = fcmp oeq float %f, 0x7FF0000000000000
ret i1 %cmp
}

define <2 x i1> @i32_vec_cast_cmp_oeq_vec_int_0_sitofp(<2 x i32> %i) {
Copy link
Member

Choose a reason for hiding this comment

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

Please pre-commit these tests.

; CHECK-LABEL: @i32_vec_cast_cmp_oeq_vec_int_0_sitofp(
; CHECK-NEXT: [[CMP:%.*]] = icmp eq <2 x i32> [[I:%.*]], zeroinitializer
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%f = sitofp <2 x i32> %i to <2 x float>
%cmp = fcmp oeq <2 x float> %f, <float 0.0, float 0.0>
ret <2 x i1> %cmp
}

define <2 x i1> @i32_vec_cast_cmp_oeq_vec_int_n0_sitofp(<2 x i32> %i) {
; CHECK-LABEL: @i32_vec_cast_cmp_oeq_vec_int_n0_sitofp(
; CHECK-NEXT: [[CMP:%.*]] = icmp eq <2 x i32> [[I:%.*]], zeroinitializer
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%f = sitofp <2 x i32> %i to <2 x float>
%cmp = fcmp oeq <2 x float> %f, <float -0.0, float -0.0>
ret <2 x i1> %cmp
}

define <2 x i1> @i32_vec_cast_cmp_oeq_vec_int_i32imax_sitofp(<2 x i32> %i) {
; CHECK-LABEL: @i32_vec_cast_cmp_oeq_vec_int_i32imax_sitofp(
; CHECK-NEXT: [[F:%.*]] = sitofp <2 x i32> [[I:%.*]] to <2 x float>
; CHECK-NEXT: [[CMP:%.*]] = fcmp oeq <2 x float> [[F]], <float 0x41E0000000000000, float 0x41E0000000000000>
; CHECK-NEXT: ret <2 x i1> [[CMP]]
;
%f = sitofp <2 x i32> %i to <2 x float>
%cmp = fcmp oeq <2 x float> %f, <float 0x41E0000000000000, float 0x41E0000000000000>
ret <2 x i1> %cmp
}

define <2 x i1> @i32_vec_cast_cmp_oeq_vec_int_negi32umax_sitofp(<2 x i32> %i) {
; CHECK-LABEL: @i32_vec_cast_cmp_oeq_vec_int_negi32umax_sitofp(
; CHECK-NEXT: ret <2 x i1> zeroinitializer
;
%f = sitofp <2 x i32> %i to <2 x float>
%cmp = fcmp oeq <2 x float> %f, <float 0xC1F0000000000000, float 0xC1F0000000000000>
ret <2 x i1> %cmp
}

define <2 x i1> @i32_vec_cast_cmp_oeq_vec_half_sitofp(<2 x i32> %i) {
; CHECK-LABEL: @i32_vec_cast_cmp_oeq_vec_half_sitofp(
; CHECK-NEXT: ret <2 x i1> zeroinitializer
;
%f = sitofp <2 x i32> %i to <2 x float>
%cmp = fcmp oeq <2 x float> %f, <float 0.5, float 0.5>
ret <2 x i1> %cmp
}

define <2 x i1> @i32_vec_cast_cmp_oeq_vec_int_inf_sitofp(<2 x i32> %i) {
; CHECK-LABEL: @i32_vec_cast_cmp_oeq_vec_int_inf_sitofp(
; CHECK-NEXT: ret <2 x i1> zeroinitializer
;
%f = sitofp <2 x i32> %i to <2 x float>
%cmp = fcmp oeq <2 x float> %f, <float 0x7FF0000000000000, float 0x7FF0000000000000>
ret <2 x i1> %cmp
}
17 changes: 17 additions & 0 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 @@ -566,3 +566,20 @@ define i32 @mixed_clamp_to_i32_2(float %x) {
%r = select i1 %lo_cmp, i32 1, i32 %i32_min
ret i32 %r
}


define <2 x float> @mixed_clamp_to_float_vec(<2 x i32> %x) {
; CHECK-LABEL: @mixed_clamp_to_float_vec(
; CHECK-NEXT: [[SI_MIN:%.*]] = call <2 x i32> @llvm.smin.v2i32(<2 x i32> [[X:%.*]], <2 x i32> <i32 255, i32 255>)
; CHECK-NEXT: [[R1:%.*]] = call <2 x i32> @llvm.smax.v2i32(<2 x i32> [[SI_MIN]], <2 x i32> <i32 1, i32 1>)
; CHECK-NEXT: [[R:%.*]] = sitofp <2 x i32> [[R1]] to <2 x float>
; CHECK-NEXT: ret <2 x float> [[R]]
;
%si_min_cmp = icmp sgt <2 x i32> %x, <i32 255, i32 255>
%si_min = select <2 x i1> %si_min_cmp, <2 x i32> <i32 255, i32 255>, <2 x i32> %x
%f_min = sitofp <2 x i32> %si_min to <2 x float>
%f_x = sitofp <2 x i32> %x to <2 x float>
%lo_cmp = fcmp ult <2 x float> %f_x, <float 1.0, float 1.0>
%r = select <2 x i1> %lo_cmp, <2 x float> <float 1.0, float 1.0>, <2 x float> %f_min
ret <2 x float> %r
}
37 changes: 37 additions & 0 deletions llvm/test/Transforms/InstCombine/sitofp.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,3 +378,40 @@ define i12 @u32_half_u12(i32 %x) {
%r = fptoui half %h to i12
ret i12 %r
}

define <2 x i1> @i8_vec_sitofp_test1(<2 x i8> %A) {
; CHECK-LABEL: @i8_vec_sitofp_test1(
; CHECK-NEXT: ret <2 x i1> <i1 true, i1 true>
;
%B = sitofp <2 x i8> %A to <2 x double>
%C = fcmp ult <2 x double> %B, <double 128.0, double 128.0>
ret <2 x i1> %C
}

define <2 x i1> @i8_vec_sitofp_test2(<2 x i8> %A) {
; CHECK-LABEL: @i8_vec_sitofp_test2(
; CHECK-NEXT: ret <2 x i1> <i1 true, i1 true>
;
%B = sitofp <2 x i8> %A to <2 x double>
%C = fcmp ugt <2 x double> %B, <double -128.1, double -128.1>
ret <2 x i1> %C
}

define <2 x i1> @i8_vec_sitofp_test3(<2 x i8> %A) {
; CHECK-LABEL: @i8_vec_sitofp_test3(
; CHECK-NEXT: ret <2 x i1> <i1 true, i1 true>
;
%B = sitofp <2 x i8> %A to <2 x double>
%C = fcmp ule <2 x double> %B, <double 127.0, double 127.0>
ret <2 x i1> %C
}

define <2 x i1> @i8_vec_sitofp_test4(<2 x i8> %A) {
; CHECK-LABEL: @i8_vec_sitofp_test4(
; CHECK-NEXT: [[C:%.*]] = icmp ne <2 x i8> [[A:%.*]], <i8 127, i8 127>
; CHECK-NEXT: ret <2 x i1> [[C]]
;
%B = sitofp <2 x i8> %A to <2 x double>
%C = fcmp ult <2 x double> %B, <double 127.0, double 127.0>
ret <2 x i1> %C
}