[llvm] [ValueTracking] Merge `cannotBeOrderedLessThanZeroImpl` into `computeKnownFPClass` (PR #76360)
Matt Arsenault via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 4 04:06:57 PST 2024
================
@@ -3737,205 +3737,6 @@ Intrinsic::ID llvm::getIntrinsicForCallSite(const CallBase &CB,
return Intrinsic::not_intrinsic;
}
-/// Deprecated, use computeKnownFPClass instead.
-///
-/// If \p SignBitOnly is true, test for a known 0 sign bit rather than a
-/// standard ordered compare. e.g. make -0.0 olt 0.0 be true because of the sign
-/// bit despite comparing equal.
-static bool cannotBeOrderedLessThanZeroImpl(const Value *V,
- const DataLayout &DL,
- const TargetLibraryInfo *TLI,
- bool SignBitOnly, unsigned Depth) {
- // TODO: This function does not do the right thing when SignBitOnly is true
- // and we're lowering to a hypothetical IEEE 754-compliant-but-evil platform
- // which flips the sign bits of NaNs. See
- // https://llvm.org/bugs/show_bug.cgi?id=31702.
-
- if (const ConstantFP *CFP = dyn_cast<ConstantFP>(V)) {
- return !CFP->getValueAPF().isNegative() ||
- (!SignBitOnly && CFP->getValueAPF().isZero());
- }
-
- // Handle vector of constants.
- if (auto *CV = dyn_cast<Constant>(V)) {
- if (auto *CVFVTy = dyn_cast<FixedVectorType>(CV->getType())) {
- unsigned NumElts = CVFVTy->getNumElements();
- for (unsigned i = 0; i != NumElts; ++i) {
- auto *CFP = dyn_cast_or_null<ConstantFP>(CV->getAggregateElement(i));
- if (!CFP)
- return false;
- if (CFP->getValueAPF().isNegative() &&
- (SignBitOnly || !CFP->getValueAPF().isZero()))
- return false;
- }
-
- // All non-negative ConstantFPs.
- return true;
- }
- }
-
- if (Depth == MaxAnalysisRecursionDepth)
- return false;
-
- const Operator *I = dyn_cast<Operator>(V);
- if (!I)
- return false;
-
- switch (I->getOpcode()) {
- default:
- break;
- // Unsigned integers are always nonnegative.
- case Instruction::UIToFP:
- return true;
- case Instruction::FDiv:
- // X / X is always exactly 1.0 or a NaN.
- if (I->getOperand(0) == I->getOperand(1) &&
- (!SignBitOnly || cast<FPMathOperator>(I)->hasNoNaNs()))
- return true;
-
- // Set SignBitOnly for RHS, because X / -0.0 is -Inf (or NaN).
- return cannotBeOrderedLessThanZeroImpl(I->getOperand(0), DL, TLI,
- SignBitOnly, Depth + 1) &&
- cannotBeOrderedLessThanZeroImpl(I->getOperand(1), DL, TLI,
- /*SignBitOnly*/ true, Depth + 1);
- case Instruction::FMul:
- // X * X is always non-negative or a NaN.
- if (I->getOperand(0) == I->getOperand(1) &&
- (!SignBitOnly || cast<FPMathOperator>(I)->hasNoNaNs()))
- return true;
-
- [[fallthrough]];
- case Instruction::FAdd:
- case Instruction::FRem:
- return cannotBeOrderedLessThanZeroImpl(I->getOperand(0), DL, TLI,
- SignBitOnly, Depth + 1) &&
- cannotBeOrderedLessThanZeroImpl(I->getOperand(1), DL, TLI,
- SignBitOnly, Depth + 1);
- case Instruction::Select:
- return cannotBeOrderedLessThanZeroImpl(I->getOperand(1), DL, TLI,
- SignBitOnly, Depth + 1) &&
- cannotBeOrderedLessThanZeroImpl(I->getOperand(2), DL, TLI,
- SignBitOnly, Depth + 1);
- case Instruction::FPExt:
- case Instruction::FPTrunc:
- // Widening/narrowing never change sign.
- return cannotBeOrderedLessThanZeroImpl(I->getOperand(0), DL, TLI,
- SignBitOnly, Depth + 1);
- case Instruction::ExtractElement:
- // Look through extract element. At the moment we keep this simple and skip
- // tracking the specific element. But at least we might find information
- // valid for all elements of the vector.
- return cannotBeOrderedLessThanZeroImpl(I->getOperand(0), DL, TLI,
- SignBitOnly, Depth + 1);
- case Instruction::Call:
- const auto *CI = cast<CallInst>(I);
- Intrinsic::ID IID = getIntrinsicForCallSite(*CI, TLI);
- switch (IID) {
- default:
- break;
- case Intrinsic::canonicalize:
- case Intrinsic::arithmetic_fence:
- case Intrinsic::floor:
- case Intrinsic::ceil:
- case Intrinsic::trunc:
- case Intrinsic::rint:
- case Intrinsic::nearbyint:
- case Intrinsic::round:
- case Intrinsic::roundeven:
- case Intrinsic::fptrunc_round:
- return cannotBeOrderedLessThanZeroImpl(I->getOperand(0), DL, TLI,
- SignBitOnly, Depth + 1);
- case Intrinsic::maxnum: {
- Value *V0 = I->getOperand(0), *V1 = I->getOperand(1);
- auto isPositiveNum = [&](Value *V) {
- if (SignBitOnly) {
- // With SignBitOnly, this is tricky because the result of
- // maxnum(+0.0, -0.0) is unspecified. Just check if the operand is
- // a constant strictly greater than 0.0.
- const APFloat *C;
- return match(V, m_APFloat(C)) &&
- *C > APFloat::getZero(C->getSemantics());
- }
-
- // -0.0 compares equal to 0.0, so if this operand is at least -0.0,
- // maxnum can't be ordered-less-than-zero.
- return isKnownNeverNaN(V, DL, TLI) &&
- cannotBeOrderedLessThanZeroImpl(V, DL, TLI, false, Depth + 1);
- };
-
- // TODO: This could be improved. We could also check that neither operand
- // has its sign bit set (and at least 1 is not-NAN?).
- return isPositiveNum(V0) || isPositiveNum(V1);
- }
-
- case Intrinsic::maximum:
- return cannotBeOrderedLessThanZeroImpl(I->getOperand(0), DL, TLI,
- SignBitOnly, Depth + 1) ||
- cannotBeOrderedLessThanZeroImpl(I->getOperand(1), DL, TLI,
- SignBitOnly, Depth + 1);
- case Intrinsic::minnum:
- case Intrinsic::minimum:
- return cannotBeOrderedLessThanZeroImpl(I->getOperand(0), DL, TLI,
- SignBitOnly, Depth + 1) &&
- cannotBeOrderedLessThanZeroImpl(I->getOperand(1), DL, TLI,
- SignBitOnly, Depth + 1);
- case Intrinsic::exp:
- case Intrinsic::exp2:
- case Intrinsic::fabs:
- return true;
- case Intrinsic::copysign:
- // Only the sign operand matters.
- return cannotBeOrderedLessThanZeroImpl(I->getOperand(1), DL, TLI, true,
- Depth + 1);
- case Intrinsic::sqrt:
- // sqrt(x) is always >= -0 or NaN. Moreover, sqrt(x) == -0 iff x == -0.
- if (!SignBitOnly)
- return true;
- return CI->hasNoNaNs() &&
- (CI->hasNoSignedZeros() ||
- cannotBeNegativeZero(CI->getOperand(0), DL, TLI));
-
- case Intrinsic::powi:
- if (ConstantInt *Exponent = dyn_cast<ConstantInt>(I->getOperand(1))) {
- // powi(x,n) is non-negative if n is even.
- if (Exponent->getBitWidth() <= 64 && Exponent->getSExtValue() % 2u == 0)
- return true;
- }
- // TODO: This is not correct. Given that exp is an integer, here are the
- // ways that pow can return a negative value:
- //
- // pow(x, exp) --> negative if exp is odd and x is negative.
- // pow(-0, exp) --> -inf if exp is negative odd.
- // pow(-0, exp) --> -0 if exp is positive odd.
- // pow(-inf, exp) --> -0 if exp is negative odd.
- // pow(-inf, exp) --> -inf if exp is positive odd.
- //
- // Therefore, if !SignBitOnly, we can return true if x >= +0 or x is NaN,
- // but we must return false if x == -0. Unfortunately we do not currently
- // have a way of expressing this constraint. See details in
- // https://llvm.org/bugs/show_bug.cgi?id=31702.
- return cannotBeOrderedLessThanZeroImpl(I->getOperand(0), DL, TLI,
- SignBitOnly, Depth + 1);
----------------
arsenm wrote:
I think a few too many things are happening at once in this patch. Most significantly, this TODO about being incorrect seems inadequately tested. I would hope to see some powi test changes showing this bug has been fixed
https://github.com/llvm/llvm-project/pull/76360
More information about the llvm-commits
mailing list