[PATCH] D149590: ValueTracking: Implement computeKnownFPClass for ldexp

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4239
+  if (!Src.isKnownNeverNegSubnormal() && Mode != DenormalMode::getIEEE())
+    KnownFPClasses |= fcNegZero;
+}
----------------
I think this needs to be `fcZero` for correctness, because a negative subnormal could be flushed to +0. (I guess this is what the TODO alludes to, but (a) it should be a FIXME and (b) maybe just do it?)


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4829-4830
+
+        // No-zero can be preserved if there's no potential for a denormal
+        // flush.
+        if (KnownSrc.isKnownNever(fcNegative))
----------------
Typo for "non-zero"? But in any case I'm not sure what the comment means or how it relates to this part of the code. Non-zeroness of the input won't be preserved if exp is negative - a non-zero input could easily underflow to a zero result.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4840
+          break;
+        if ((KnownSrc.KnownFPClasses & ExpInfoMask) == fcNone)
+          break;
----------------



================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4849
+        if (ExpKnownBits.isZero()) {
+          // ldexp(x, 0) -> x, so propagate everything.
+          Known.propagateCanonicalizingSrc(KnownSrc, *II->getFunction(),
----------------
This should have been simplified already to an explicit canonicalize, right?


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

https://reviews.llvm.org/D149590