[PATCH] D157030: InstCombine: Fold out scale-if-denormal pattern

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineInternal.h:237
+
+  /// Check if fmul \p MulVal, +0.0 will yield +0.0
+  bool fmulByZeroIsZero(Value *MulVal, FastMathFlags FMF,
----------------
Seems a bit strong to say that `fmul nsz MulVal, +0.0` **will** yield +0.0 when MulVal is negative. I think the nsz flag just means that optimizations applied to this instruction don't have to respect the sign of zero. It's still possible that some other later (non-nsz) instruction would observe a -0.0 result from this fmul.

I think the behaviour is fine but the comment could use improving.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:3413
+      // TODO: Could relax the signed zero logic. We just need to know the sign
+      // of the result matches, so if we know fmul x, y and has the same sign as
+      // x.
----------------
Stray "and"?


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

https://reviews.llvm.org/D157030