Skip to content

Commit

Permalink
Revert "InstSimplify: Use correct interested FP classes when simplify…
Browse files Browse the repository at this point in the history 
…ing fcmp"

Revert "InstSimplify: Add baseline tests for reported regression"
Revert "InstSimplify: Start cleaning up simplifyFCmpInst"

This reverts commit 0637b00.
This reverts commit 239fb20.
This reverts commit ddb3f12.

These commits causes crashes when compiling chromium code, attached reduced ir at: https://reviews.llvm.org/D151887#4634914
  • Loading branch information
@ZequanWu
ZequanWu committed Sep 1, 2023
1 parent 471004c commit 89f0314
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 235 deletions.
27 changes: 0 additions & 27 deletions llvm/include/llvm/Analysis/ValueTracking.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "llvm/ADT/SmallSet.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/FMF.h"
#include "llvm/IR/InstrTypes.h"
#include "llvm/IR/Intrinsics.h"
#include <cassert>
Expand Down Expand Up @@ -230,10 +229,6 @@ std::pair<Value *, FPClassTest> fcmpToClassTest(CmpInst::Predicate Pred,
const Function &F, Value *LHS,
Value *RHS,
bool LookThroughSrc = true);
std::pair<Value *, FPClassTest> fcmpToClassTest(CmpInst::Predicate Pred,
const Function &F, Value *LHS,
const APFloat *ConstRHS,
bool LookThroughSrc = true);

struct KnownFPClass {
/// Floating-point classes the value could be one of.
Expand Down Expand Up @@ -476,28 +471,6 @@ KnownFPClass computeKnownFPClass(
const Instruction *CxtI = nullptr, const DominatorTree *DT = nullptr,
bool UseInstrInfo = true);

/// Wrapper to account for known fast math flags at the use instruction.
inline KnownFPClass computeKnownFPClass(
const Value *V, FastMathFlags FMF, const DataLayout &DL,
FPClassTest InterestedClasses = fcAllFlags, unsigned Depth = 0,
const TargetLibraryInfo *TLI = nullptr, AssumptionCache *AC = nullptr,
const Instruction *CxtI = nullptr, const DominatorTree *DT = nullptr,
bool UseInstrInfo = true) {
if (FMF.noNaNs())
InterestedClasses &= ~fcNan;
if (FMF.noInfs())
InterestedClasses &= ~fcInf;

KnownFPClass Result = computeKnownFPClass(V, DL, InterestedClasses, Depth,
TLI, AC, CxtI, DT, UseInstrInfo);

if (FMF.noNaNs())
Result.KnownFPClasses &= ~fcNan;
if (FMF.noInfs())
Result.KnownFPClasses &= ~fcInf;
return Result;
}

/// Return true if we can prove that the specified FP value is never equal to
/// -0.0. Users should use caution when considering PreserveSign
/// denormal-fp-math.
Expand Down
142 changes: 69 additions & 73 deletions llvm/lib/Analysis/InstructionSimplify.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4060,6 +4060,19 @@ static Value *simplifyFCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
if (Pred == FCmpInst::FCMP_TRUE)
return getTrue(RetTy);

// Fold (un)ordered comparison if we can determine there are no NaNs.
if (Pred == FCmpInst::FCMP_UNO || Pred == FCmpInst::FCMP_ORD)
if (FMF.noNaNs() ||
(isKnownNeverNaN(LHS, Q.DL, Q.TLI, 0, Q.AC, Q.CxtI, Q.DT) &&
isKnownNeverNaN(RHS, Q.DL, Q.TLI, 0, Q.AC, Q.CxtI, Q.DT)))
return ConstantInt::get(RetTy, Pred == FCmpInst::FCMP_ORD);

// NaN is unordered; NaN is not ordered.
assert((FCmpInst::isOrdered(Pred) || FCmpInst::isUnordered(Pred)) &&
"Comparison must be either ordered or unordered");
if (match(RHS, m_NaN()))
return ConstantInt::get(RetTy, CmpInst::isUnordered(Pred));

// fcmp pred x, poison and fcmp pred poison, x
// fold to poison
if (isa<PoisonValue>(LHS) || isa<PoisonValue>(RHS))
Expand All @@ -4081,86 +4094,80 @@ static Value *simplifyFCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
return getFalse(RetTy);
}

// Fold (un)ordered comparison if we can determine there are no NaNs.
//
// This catches the 2 variable input case, constants are handled below as a
// class-like compare.
if (Pred == FCmpInst::FCMP_ORD || Pred == FCmpInst::FCMP_UNO) {
if (FMF.noNaNs() ||
(isKnownNeverNaN(RHS, Q.DL, Q.TLI, 0, Q.AC, Q.CxtI, Q.DT) &&
isKnownNeverNaN(LHS, Q.DL, Q.TLI, 0, Q.AC, Q.CxtI, Q.DT)))
return ConstantInt::get(RetTy, Pred == FCmpInst::FCMP_ORD);
}

const APFloat *C = nullptr;
match(RHS, m_APFloatAllowUndef(C));
std::optional<KnownFPClass> FullKnownClassLHS;

// Lazily compute the possible classes for LHS. Avoid computing it twice if
// RHS is a 0.
auto computeLHSClass = [=, &FullKnownClassLHS](FPClassTest InterestedFlags =
fcAllFlags) {
if (FullKnownClassLHS)
return *FullKnownClassLHS;
return computeKnownFPClass(LHS, FMF, Q.DL, InterestedFlags, 0, Q.TLI, Q.AC,
Q.CxtI, Q.DT, Q.IIQ.UseInstrInfo);
};
// Handle fcmp with constant RHS.
// TODO: Use match with a specific FP value, so these work with vectors with
// undef lanes.
const APFloat *C;
if (match(RHS, m_APFloat(C))) {
// Check whether the constant is an infinity.
if (C->isInfinity()) {
if (C->isNegative()) {
switch (Pred) {
case FCmpInst::FCMP_OLT:
// No value is ordered and less than negative infinity.
return getFalse(RetTy);
case FCmpInst::FCMP_UGE:
// All values are unordered with or at least negative infinity.
return getTrue(RetTy);
default:
break;
}
} else {
switch (Pred) {
case FCmpInst::FCMP_OGT:
// No value is ordered and greater than infinity.
return getFalse(RetTy);
case FCmpInst::FCMP_ULE:
// All values are unordered with and at most infinity.
return getTrue(RetTy);
default:
break;
}
}

if (C && Q.CxtI) {
// Fold out compares that express a class test.
//
// FIXME: Should be able to perform folds without context
// instruction. Always pass in the context function?

const Function *ParentF = Q.CxtI->getFunction();
auto [ClassVal, ClassTest] = fcmpToClassTest(Pred, *ParentF, LHS, C);
if (ClassVal) {
FullKnownClassLHS = computeLHSClass();
if ((FullKnownClassLHS->KnownFPClasses & ClassTest) == fcNone)
// LHS == Inf
if (Pred == FCmpInst::FCMP_OEQ &&
isKnownNeverInfinity(LHS, Q.DL, Q.TLI, 0, Q.AC, Q.CxtI, Q.DT))
return getFalse(RetTy);
// LHS != Inf
if (Pred == FCmpInst::FCMP_UNE &&
isKnownNeverInfinity(LHS, Q.DL, Q.TLI, 0, Q.AC, Q.CxtI, Q.DT))
return getTrue(RetTy);
// LHS == Inf || LHS == NaN
if (Pred == FCmpInst::FCMP_UEQ &&
isKnownNeverInfOrNaN(LHS, Q.DL, Q.TLI, 0, Q.AC, Q.CxtI, Q.DT))
return getFalse(RetTy);
if ((FullKnownClassLHS->KnownFPClasses & ~ClassTest) == fcNone)
// LHS != Inf && LHS != NaN
if (Pred == FCmpInst::FCMP_ONE &&
isKnownNeverInfOrNaN(LHS, Q.DL, Q.TLI, 0, Q.AC, Q.CxtI, Q.DT))
return getTrue(RetTy);
}
}

// Handle fcmp with constant RHS.
if (C) {
// TODO: Need version fcmpToClassTest which returns implied class when the
// compare isn't a complete class test. e.g. > 1.0 implies fcPositive, but
// isn't implementable as a class call.
if (C->isNegative() && !C->isNegZero()) {
FPClassTest Interested = KnownFPClass::OrderedLessThanZeroMask;

// FIXME: This assert won't always hold if we depend on the context
// instruction above
assert(!C->isNaN() && "Unexpected NaN constant!");
// TODO: We can catch more cases by using a range check rather than
// relying on CannotBeOrderedLessThanZero.
switch (Pred) {
case FCmpInst::FCMP_UGE:
case FCmpInst::FCMP_UGT:
case FCmpInst::FCMP_UNE: {
KnownFPClass KnownClass = computeLHSClass(Interested);

case FCmpInst::FCMP_UNE:
// (X >= 0) implies (X > C) when (C < 0)
if (KnownClass.cannotBeOrderedLessThanZero())
if (cannotBeOrderedLessThanZero(LHS, Q.DL, Q.TLI, 0,
Q.AC, Q.CxtI, Q.DT))
return getTrue(RetTy);
break;
}
case FCmpInst::FCMP_OEQ:
case FCmpInst::FCMP_OLE:
case FCmpInst::FCMP_OLT: {
KnownFPClass KnownClass = computeLHSClass(Interested);

case FCmpInst::FCMP_OLT:
// (X >= 0) implies !(X < C) when (C < 0)
if (KnownClass.cannotBeOrderedLessThanZero())
if (cannotBeOrderedLessThanZero(LHS, Q.DL, Q.TLI, 0, Q.AC, Q.CxtI,
Q.DT))
return getFalse(RetTy);
break;
}
default:
break;
}
}

// Check comparison of [minnum/maxnum with constant] with other constant.
const APFloat *C2;
if ((match(LHS, m_Intrinsic<Intrinsic::minnum>(m_Value(), m_APFloat(C2))) &&
Expand Down Expand Up @@ -4207,17 +4214,13 @@ static Value *simplifyFCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
}
}

// TODO: Could fold this with above if there were a matcher which returned all
// classes in a non-splat vector.
if (match(RHS, m_AnyZeroFP())) {
switch (Pred) {
case FCmpInst::FCMP_OGE:
case FCmpInst::FCMP_ULT: {
FPClassTest Interested = KnownFPClass::OrderedLessThanZeroMask;
if (!FMF.noNaNs())
Interested |= fcNan;

KnownFPClass Known = computeLHSClass(Interested);
FPClassTest Interested = FMF.noNaNs() ? fcNegative : fcNegative | fcNan;
KnownFPClass Known = computeKnownFPClass(LHS, Q.DL, Interested, 0,
Q.TLI, Q.AC, Q.CxtI, Q.DT);

// Positive or zero X >= 0.0 --> true
// Positive or zero X < 0.0 --> false
Expand All @@ -4227,16 +4230,12 @@ static Value *simplifyFCmpInst(unsigned Predicate, Value *LHS, Value *RHS,
break;
}
case FCmpInst::FCMP_UGE:
case FCmpInst::FCMP_OLT: {
FPClassTest Interested = KnownFPClass::OrderedLessThanZeroMask;
KnownFPClass Known = computeLHSClass(Interested);

case FCmpInst::FCMP_OLT:
// Positive or zero or nan X >= 0.0 --> true
// Positive or zero or nan X < 0.0 --> false
if (Known.cannotBeOrderedLessThanZero())
if (cannotBeOrderedLessThanZero(LHS, Q.DL, Q.TLI, 0, Q.AC, Q.CxtI, Q.DT))
return Pred == FCmpInst::FCMP_UGE ? getTrue(RetTy) : getFalse(RetTy);
break;
}
default:
break;
}
Expand Down Expand Up @@ -6817,9 +6816,6 @@ static Value *simplifyInstructionWithOperands(Instruction *I,
const SimplifyQuery &SQ,
unsigned MaxRecurse) {
assert(I->getFunction() && "instruction should be inserted in a function");
assert((!SQ.CxtI || SQ.CxtI->getFunction() == I->getFunction()) &&
"context instruction should be in the same function");

const SimplifyQuery Q = SQ.CxtI ? SQ : SQ.getWithInstruction(I);

switch (I->getOpcode()) {
Expand Down
8 changes: 1 addition & 7 deletions llvm/lib/Analysis/ValueTracking.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4007,15 +4007,9 @@ std::pair<Value *, FPClassTest> llvm::fcmpToClassTest(FCmpInst::Predicate Pred,
Value *LHS, Value *RHS,
bool LookThroughSrc) {
const APFloat *ConstRHS;
if (!match(RHS, m_APFloatAllowUndef(ConstRHS)))
if (!match(RHS, m_APFloat(ConstRHS)))
return {nullptr, fcNone};

return fcmpToClassTest(Pred, F, LHS, ConstRHS, LookThroughSrc);
}

std::pair<Value *, FPClassTest>
llvm::fcmpToClassTest(FCmpInst::Predicate Pred, const Function &F, Value *LHS,
const APFloat *ConstRHS, bool LookThroughSrc) {
// fcmp ord x, zero|normal|subnormal|inf -> ~fcNan
if (Pred == FCmpInst::FCMP_ORD && !ConstRHS->isNaN())
return {LHS, ~fcNan};
Expand Down
3 changes: 2 additions & 1 deletion llvm/test/Transforms/Attributor/nofpclass.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,8 @@ define half @assume_fcmp_fabs_with_other_fabs_assume_fallback(half %arg) {
; CHECK-NEXT: call void @llvm.assume(i1 noundef true) #[[ATTR16]]
; CHECK-NEXT: [[UNRELATED_FABS:%.*]] = fcmp oeq half [[FABS]], 0xH0000
; CHECK-NEXT: call void @llvm.assume(i1 noundef [[UNRELATED_FABS]]) #[[ATTR16]]
; CHECK-NEXT: call void @llvm.assume(i1 noundef true) #[[ATTR16]]
; CHECK-NEXT: [[IS_SUBNORMAL:%.*]] = fcmp olt half [[FABS]], 0xH0400
; CHECK-NEXT: call void @llvm.assume(i1 noundef [[IS_SUBNORMAL]]) #[[ATTR16]]
; CHECK-NEXT: call void @extern.use.f16(half nofpclass(nan inf norm) [[ARG]])
; CHECK-NEXT: call void @extern.use.f16(half nofpclass(nan inf nzero sub norm) [[FABS]])
; CHECK-NEXT: ret half [[ARG]]
Expand Down
8 changes: 6 additions & 2 deletions llvm/test/Transforms/InstCombine/fcmp.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,9 @@ define i1 @is_signbit_clear_nonzero(double %x) {

define i1 @is_signbit_set_simplify_zero(double %x) {
; CHECK-LABEL: @is_signbit_set_simplify_zero(
; CHECK-NEXT: ret i1 false
; CHECK-NEXT: [[S:%.*]] = call double @llvm.copysign.f64(double 0.000000e+00, double [[X:%.*]])
; CHECK-NEXT: [[R:%.*]] = fcmp ogt double [[S]], 0.000000e+00
; CHECK-NEXT: ret i1 [[R]]
;
%s = call double @llvm.copysign.f64(double 0.0, double %x)
%r = fcmp ogt double %s, 0.0
Expand All @@ -729,7 +731,9 @@ define i1 @is_signbit_set_simplify_zero(double %x) {

define i1 @is_signbit_set_simplify_nan(double %x) {
; CHECK-LABEL: @is_signbit_set_simplify_nan(
; CHECK-NEXT: ret i1 false
; CHECK-NEXT: [[S:%.*]] = call double @llvm.copysign.f64(double 0xFFFFFFFFFFFFFFFF, double [[X:%.*]])
; CHECK-NEXT: [[R:%.*]] = fcmp ogt double [[S]], 0.000000e+00
; CHECK-NEXT: ret i1 [[R]]
;
%s = call double @llvm.copysign.f64(double 0xffffffffffffffff, double %x)
%r = fcmp ogt double %s, 0.0
Expand Down
12 changes: 8 additions & 4 deletions llvm/test/Transforms/InstCombine/is_fpclass.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2444,7 +2444,8 @@ define <2 x i1> @test_class_fneg_fabs_posinf_negnormal_possubnormal_negzero_nan_

define i1 @test_class_is_zero_nozero_src(float nofpclass(zero) %arg) {
; CHECK-LABEL: @test_class_is_zero_nozero_src(
; CHECK-NEXT: ret i1 false
; CHECK-NEXT: [[CLASS:%.*]] = fcmp oeq float [[ARG:%.*]], 0.000000e+00
; CHECK-NEXT: ret i1 [[CLASS]]
;
%class = call i1 @llvm.is.fpclass.f32(float %arg, i32 96)
ret i1 %class
Expand Down Expand Up @@ -2577,7 +2578,8 @@ define i1 @test_class_is_neginf_or_nopinf_src(float nofpclass(pinf) %arg) {

define i1 @test_class_is_neginf_noninf_src(float nofpclass(ninf) %arg) {
; CHECK-LABEL: @test_class_is_neginf_noninf_src(
; CHECK-NEXT: ret i1 false
; CHECK-NEXT: [[CLASS:%.*]] = fcmp oeq float [[ARG:%.*]], 0xFFF0000000000000
; CHECK-NEXT: ret i1 [[CLASS]]
;
%class = call i1 @llvm.is.fpclass.f32(float %arg, i32 4)
ret i1 %class
Expand All @@ -2602,7 +2604,8 @@ define i1 @test_class_is_posinf_noninf_src(float nofpclass(ninf) %arg) {

define i1 @test_class_is_posinf_nopinf_src(float nofpclass(pinf) %arg) {
; CHECK-LABEL: @test_class_is_posinf_nopinf_src(
; CHECK-NEXT: ret i1 false
; CHECK-NEXT: [[CLASS:%.*]] = fcmp oeq float [[ARG:%.*]], 0x7FF0000000000000
; CHECK-NEXT: ret i1 [[CLASS]]
;
%class = call i1 @llvm.is.fpclass.f32(float %arg, i32 512)
ret i1 %class
Expand Down Expand Up @@ -2730,7 +2733,8 @@ define i1 @test_class_is_nan_assume_uno(float %x) {
; CHECK-LABEL: @test_class_is_nan_assume_uno(
; CHECK-NEXT: [[ORD:%.*]] = fcmp uno float [[X:%.*]], 0.000000e+00
; CHECK-NEXT: call void @llvm.assume(i1 [[ORD]])
; CHECK-NEXT: ret i1 true
; CHECK-NEXT: [[CLASS:%.*]] = fcmp uno float [[X]], 0.000000e+00
; CHECK-NEXT: ret i1 [[CLASS]]
;
%ord = fcmp uno float %x, 0.0
call void @llvm.assume(i1 %ord)
Expand Down

0 comments on commit 89f0314

Please sign in to comment.