[PATCH] D28928: [ValueTracking] Implement SignBitMustBeZero correctly for sqrt.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:2673
+      return CI->hasNoNaNs() && (CI->hasNoSignedZeros() ||
+                                 CannotBeNegativeZero(CI->getOperand(0), TLI));
+
----------------
You need to prove that the result isn't a NaN and isn't -0.  Using nsz to prove the result isn't -0 is suspicious.  For nsz, LangRef says "Allow optimizations to treat the sign of a zero argument or result as insignificant.", so the operand could in fact be -0.  Actually, sqrt(0) could produce -0 under that definition.

Really, what we want to do is propagate in the fast-math flags from caller; we want to transform nnan nsz fabs(sqrt(x)) -> sqrt(x); that transform depends on the fast-math flags of the fabs, not the fast-math flags of the sqrt().

I think the root of this mess is "SignBitOnly": it doesn't really capture what we care about.  We can split floating-point numbers into categories: +inf, -inf, +0, -0, positive finite, negative finite, negative nan, and positive nan.  CannotBeOrderedLessThanZero needs to prove that the input is not in the category -inf or the category negative finite.  SignBitMustBeZero needs to prove that the input is not in one of the categories -0, -inf, negative finite, or negative nan.  To transform nnan fabs(x) -> x, we need to prove that the input is not in one of the categories -0, -inf, or negative finite.  To transform nnan nsz fabs(x) -> x, we need to prove that the input is not in one of the categories -inf, or negative finite.  To transform fast fabs(x) -> x, we need to prove the input is not in the category negative finite.

Does this make sense, or is it completely crazy? :)


https://reviews.llvm.org/D28928