[PATCH] D154735: ValueTracking: ldexp cannot return denormals based on range of exponent

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4708
 
-        const Function *F = II->getFunction();
+        const int MantissaBits = Precision - 1; // Subtract sign bit.
+        if (ExpRange.getLower().sge(static_cast<int64_t>(MantissaBits)))
----------------
I don't think "sign bit" is relevant here. I guess you're subtracting 1 because you're not interested in the "implicit bit" at the front of the mantissa.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4709
+        const int MantissaBits = Precision - 1; // Subtract sign bit.
+        if (ExpRange.getLower().sge(static_cast<int64_t>(MantissaBits)))
+          Known.knownNot(fcSubnormal);
----------------
Ranges can wrap, so getLower() is not necessarily the smallest value in the range.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4714
+        const APInt *ConstVal = ExpRange.getSingleElement();
+        if (ConstVal && ConstVal->isZero()) {
           // ldexp(x, 0) -> x, so propagate everything.
----------------
Why do you have to handle this case here? Surely something else should already have simplified the operand to an actual constant?


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4719
+        } else if (ExpRange.isAllNegative()) {
           // If we know the power is < 0, can't introduce inf
           if (KnownSrc.isKnownNeverPosInfinity())
----------------
Could relax this to "power is <= 0", but there is no handy helper for that.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D154735/new/

https://reviews.llvm.org/D154735