Skip to content

Optimize fptrunc(x)>=C1 --> x>=C2 #99475

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

Merged
merged 3 commits into from
Jul 19, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
99 changes: 99 additions & 0 deletions llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8031,6 +8031,101 @@ static Instruction *foldFCmpReciprocalAndZero(FCmpInst &I, Instruction *LHSI,
return new FCmpInst(Pred, LHSI->getOperand(1), RHSC, "", &I);
}


// Fold fptrunc(x) < constant --> x < constant if possible.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Fold fptrunc(x) < constant --> x < constant if possible.
/// Fold fptrunc(x) < constant --> x < constant if possible.

This also covers other compare types, can you express that in the comment

static Instruction *foldFCmpFpTrunc(FCmpInst &I, Instruction *LHSI,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static Instruction *foldFCmpFpTrunc(FCmpInst &I, Instruction *LHSI,
static Instruction *foldFCmpFpTrunc(FCmpInst &I, const Instruction &FPTrunc,

Constant *RHSC) {
FCmpInst::Predicate Pred = I.getPredicate();
bool RoundDown = false;

if (Pred == FCmpInst::FCMP_OGE || Pred == FCmpInst::FCMP_UGE ||
Pred == FCmpInst::FCMP_OLT || Pred == FCmpInst::FCMP_ULT)
RoundDown = true;
else if (Pred == FCmpInst::FCMP_OGT || Pred == FCmpInst::FCMP_UGT ||
Pred == FCmpInst::FCMP_OLE || Pred == FCmpInst::FCMP_ULE)
RoundDown = false;
else
Comment on lines +8050 to +8056
Copy link
Contributor

Choose a reason for hiding this comment

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

uge, ule cases not tested. Plus negative test for others

return nullptr;

const APFloat *RValue;
if (!match(RHSC, m_APFloat(RValue)))
return nullptr;

// RHSC should not be nan or infinity.
if (RValue->isNaN() || RValue->isInfinity())
return nullptr;

Type *LType = LHSI->getOperand(0)->getType();
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems more like a "Src" than a "Left" type

Type *RType = RHSC->getType();
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems more like a "Dest" than a "Right" type

Type *LEleType = LType->getScalarType();
Type *REleType = RType->getScalarType();

APFloat NextRValue = *RValue;
NextRValue.next(RoundDown);

// Promote 'RValue' and 'NextRValue' to 'LType'.
APFloat ExtRValue = *RValue;
APFloat ExtNextRValue = NextRValue;
bool lossInfo;
ExtRValue.convert(LEleType->getFltSemantics(), APFloat::rmNearestTiesToEven,
&lossInfo);
ExtNextRValue.convert(LEleType->getFltSemantics(),
APFloat::rmNearestTiesToEven, &lossInfo);

// The (negative) maximum of 'RValue' may become infinity when rounded up
// (down). Set the limit of 'ExtNextRValue'.
if (NextRValue.isInfinity())
ExtNextRValue = scalbn(ExtRValue, 1, APFloat::rmNearestTiesToEven);

// Binary search to find the maximal (or minimal) value after 'RValue'
// promotion. 'RValue' should obey normal comparison rules, which means nan or
// inf is not allowed here.
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this comment down to the loop

APFloat RoundValue{LEleType->getFltSemantics()};
Copy link
Contributor

Choose a reason for hiding this comment

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

Move this down to where it's used before the loop


APFloat LowBound = RoundDown ? ExtNextRValue : ExtRValue;
APFloat UpBound = RoundDown ? ExtRValue : ExtNextRValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
APFloat LowBound = RoundDown ? ExtNextRValue : ExtRValue;
APFloat UpBound = RoundDown ? ExtRValue : ExtNextRValue;
const APFloat &LowBound = RoundDown ? ExtNextRValue : ExtRValue;
const APFloat &UpBound = RoundDown ? ExtRValue : ExtNextRValue;


auto IsRoundingFound = [](const APFloat &LowBound, const APFloat &UpBound) {
APFloat UpBoundNext = UpBound;
UpBoundNext.next(true);
return LowBound == UpBoundNext;
};

auto EqualRValueAfterTrunc = [&](const APFloat &ExtValue) {
APFloat TruncValue = ExtValue;
TruncValue.convert(REleType->getFltSemantics(),
APFloat::rmNearestTiesToEven, &lossInfo);
return TruncValue == *RValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do these compares amount to the same as directly using losesInfo?

};

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this should be a directly computable constant without a binary search

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, binary search can be replaced by average value for most cases, and I got confused before. However, it seems a little tricky to tackle the comparison with the largest value. I tried several ways to work around, but all are unsatisfactory. Might be late for the final solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although the absence of next(max_float) which is infinity actually, we can image its existence in wider types, and it should equal to max_float + (max_float - prev(max_float)) in most cases (if not all). A bound check can ensure its correctness.

This method balance the efficiency and correctness, and keep consistence with logics for other floats despite the fact that it might not be standard for all float types.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you planning on switching the patch to do this? Needs comments either way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to, but it seems that the rule can't be applied to ppc_fp128 when converted from max_float due to unknown precision loss. I want more investigation before decision

while (true) {
// Finish searching when 'LowBound' is next to 'UpBound'.
if (IsRoundingFound(LowBound, UpBound)) {
RoundValue = RoundDown ? UpBound : LowBound;
break;
}

APFloat Mid = scalbn(LowBound + UpBound, -1, APFloat::rmNearestTiesToEven);
bool EqualRValue = EqualRValueAfterTrunc(Mid);

// 'EqualRValue' indicates whether Mid is qualified to be the final round
// value. if 'EqualRValue' == true, 'Mid' might be the final round value
// if 'RoundDown' == true, 'UpBound' can't be the final round value
// if 'RoudnDown' == false, 'DownBound' can't be the final round value
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// if 'RoudnDown' == false, 'DownBound' can't be the final round value
// if 'RoundDown' == false, 'DownBound' can't be the final round value

// if 'EqualRValue' == false, 'Mid' can't be the final round value
// if 'RoundDown' == true, 'DownBound' can't be the final round value
// if 'RoundDown' == false, 'UpBound' can't be the final round value
if (EqualRValue == RoundDown) {
UpBound = Mid;
} else {
LowBound = Mid;
}
}

return new FCmpInst(Pred, LHSI->getOperand(0),
ConstantFP::get(LType, RoundValue), "", &I);
}

/// Optimize fabs(X) compared with zero.
static Instruction *foldFabsWithFcmpZero(FCmpInst &I, InstCombinerImpl &IC) {
Value *X;
Expand Down Expand Up @@ -8522,6 +8617,10 @@ Instruction *InstCombinerImpl::visitFCmpInst(FCmpInst &I) {
cast<LoadInst>(LHSI), GEP, GV, I))
return Res;
break;
case Instruction::FPTrunc:
if (Instruction *NV = foldFCmpFpTrunc(I, LHSI, RHSC))
return NV;
break;
}
}

Expand Down
Loading