Skip to content

Commit

Permalink
[InstCombine] Fold select based logic of fcmps with same operands whe…
Browse files Browse the repository at this point in the history
…n FMF is present.

If we have a logical and/or in select form and the true/false operand
is an fcmp with poison generating FMF, we won't be able to fold it
to an and/or instruction. This prevents us from optimizing the case
where it is a logical operation of two fcmps with identical operands.

This patch adds explicit checks for this case that doesn't rely on
converting to and/or to do the optimization. It reuses the existing
foldLogicOfFCmps, but adds a new flag to disable the other combine
that is inside that function.

FMF flags from the two FCmps are intersected using the logic added in
D121243. The FIXME has been updated to indicate that we can only use
a union for the non-select form.

This allows us to optimize cases like this from compare-fp-3.c in the
gcc torture suite with fast math.

void
test1 (float x, float y)
{
  if ((x==y) && (x!=y))
    link_error0();
}

Reviewed By: spatel

Differential Revision: https://reviews.llvm.org/D121323
  • Loading branch information
topperc committed Mar 14, 2022
1 parent 236695e commit ce78e68
Show file tree
Hide file tree
Showing 5 changed files with 2,931 additions and 21 deletions.
15 changes: 9 additions & 6 deletions llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
Expand Up @@ -1322,7 +1322,7 @@ Value *InstCombinerImpl::foldAndOfICmps(ICmpInst *LHS, ICmpInst *RHS,
}

Value *InstCombinerImpl::foldLogicOfFCmps(FCmpInst *LHS, FCmpInst *RHS,
bool IsAnd) {
bool IsAnd, bool IsLogicalSelect) {
Value *LHS0 = LHS->getOperand(0), *LHS1 = LHS->getOperand(1);
Value *RHS0 = RHS->getOperand(0), *RHS1 = RHS->getOperand(1);
FCmpInst::Predicate PredL = LHS->getPredicate(), PredR = RHS->getPredicate();
Expand Down Expand Up @@ -1353,7 +1353,7 @@ Value *InstCombinerImpl::foldLogicOfFCmps(FCmpInst *LHS, FCmpInst *RHS,
unsigned NewPred = IsAnd ? FCmpCodeL & FCmpCodeR : FCmpCodeL | FCmpCodeR;

// Intersect the fast math flags.
// TODO: We can union the fast math flags.
// TODO: We can union the fast math flags unless this is a logical select.
IRBuilder<>::FastMathFlagGuard FMFG(Builder);
FastMathFlags FMF = LHS->getFastMathFlags();
FMF &= RHS->getFastMathFlags();
Expand All @@ -1362,8 +1362,11 @@ Value *InstCombinerImpl::foldLogicOfFCmps(FCmpInst *LHS, FCmpInst *RHS,
return getFCmpValue(NewPred, LHS0, LHS1, Builder);
}

if ((PredL == FCmpInst::FCMP_ORD && PredR == FCmpInst::FCMP_ORD && IsAnd) ||
(PredL == FCmpInst::FCMP_UNO && PredR == FCmpInst::FCMP_UNO && !IsAnd)) {
// This transform is not valid for a logical select.
if (!IsLogicalSelect &&
((PredL == FCmpInst::FCMP_ORD && PredR == FCmpInst::FCMP_ORD && IsAnd) ||
(PredL == FCmpInst::FCMP_UNO && PredR == FCmpInst::FCMP_UNO &&
!IsAnd))) {
if (LHS0->getType() != RHS0->getType())
return nullptr;

Expand Down Expand Up @@ -2125,7 +2128,7 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) {

if (FCmpInst *LHS = dyn_cast<FCmpInst>(I.getOperand(0)))
if (FCmpInst *RHS = dyn_cast<FCmpInst>(I.getOperand(1)))
if (Value *Res = foldLogicOfFCmps(LHS, RHS, true))
if (Value *Res = foldLogicOfFCmps(LHS, RHS, /*IsAnd*/ true))
return replaceInstUsesWith(I, Res);

if (Instruction *FoldedFCmps = reassociateFCmps(I, Builder))
Expand Down Expand Up @@ -2891,7 +2894,7 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {

if (FCmpInst *LHS = dyn_cast<FCmpInst>(I.getOperand(0)))
if (FCmpInst *RHS = dyn_cast<FCmpInst>(I.getOperand(1)))
if (Value *Res = foldLogicOfFCmps(LHS, RHS, false))
if (Value *Res = foldLogicOfFCmps(LHS, RHS, /*IsAnd*/ false))
return replaceInstUsesWith(I, Res);

if (Instruction *FoldedFCmps = reassociateFCmps(I, Builder))
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Transforms/InstCombine/InstCombineInternal.h
Expand Up @@ -353,7 +353,8 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
/// Optimize (fcmp)&(fcmp) or (fcmp)|(fcmp).
/// NOTE: Unlike most of instcombine, this returns a Value which should
/// already be inserted into the function.
Value *foldLogicOfFCmps(FCmpInst *LHS, FCmpInst *RHS, bool IsAnd);
Value *foldLogicOfFCmps(FCmpInst *LHS, FCmpInst *RHS, bool IsAnd,
bool IsLogicalSelect = false);

Value *foldAndOrOfICmpsOfAndWithPow2(ICmpInst *LHS, ICmpInst *RHS,
Instruction *CxtI, bool IsAnd,
Expand Down
28 changes: 22 additions & 6 deletions llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp
Expand Up @@ -2605,13 +2605,29 @@ Instruction *InstCombinerImpl::visitSelectInst(SelectInst &SI) {
// Folding select to and/or i1 isn't poison safe in general. impliesPoison
// checks whether folding it does not convert a well-defined value into
// poison.
if (match(TrueVal, m_One()) && impliesPoison(FalseVal, CondVal)) {
// Change: A = select B, true, C --> A = or B, C
return BinaryOperator::CreateOr(CondVal, FalseVal);
if (match(TrueVal, m_One())) {
if (impliesPoison(FalseVal, CondVal)) {
// Change: A = select B, true, C --> A = or B, C
return BinaryOperator::CreateOr(CondVal, FalseVal);
}

if (auto *LHS = dyn_cast<FCmpInst>(CondVal))
if (auto *RHS = dyn_cast<FCmpInst>(FalseVal))
if (Value *V = foldLogicOfFCmps(LHS, RHS, /*IsAnd*/ false,
/*IsSelectLogical*/ true))
return replaceInstUsesWith(SI, V);
}
if (match(FalseVal, m_Zero()) && impliesPoison(TrueVal, CondVal)) {
// Change: A = select B, C, false --> A = and B, C
return BinaryOperator::CreateAnd(CondVal, TrueVal);
if (match(FalseVal, m_Zero())) {
if (impliesPoison(TrueVal, CondVal)) {
// Change: A = select B, C, false --> A = and B, C
return BinaryOperator::CreateAnd(CondVal, TrueVal);
}

if (auto *LHS = dyn_cast<FCmpInst>(CondVal))
if (auto *RHS = dyn_cast<FCmpInst>(TrueVal))
if (Value *V = foldLogicOfFCmps(LHS, RHS, /*IsAnd*/ true,
/*IsSelectLogical*/ true))
return replaceInstUsesWith(SI, V);
}

auto *One = ConstantInt::getTrue(SelType);
Expand Down

0 comments on commit ce78e68

Please sign in to comment.