[PATCH] D151887: InstSimplify: Start cleaning up simplifyFCmpInst

[Joshua Cranmer via Phabricator via llvm-commits]

jcranmer-intel added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4096
+
+  if (C && Q.CxtI) {
+    // FIXME: Should be able to perform folds without context
----------------
Nit: this if statement needs a comment explaining which cases its intended to case (compares that can be written as class tests.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4112-4113
+  // Handle fcmp with constant RHS.
+  // TODO: Use match with a specific FP value, so these work with vectors with
+  // undef lanes.
+  if (C) {
----------------
This TODO is effectively done with m_APFloatAllowUndef, right?


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4119
     if (C->isNegative() && !C->isNegZero()) {
-      assert(!C->isNaN() && "Unexpected NaN constant!");
+      FPClassTest Interested = FMF.noNaNs() ? fcPositive : fcPositive | fcNan;
+
----------------
The clearing of fcNaN is already done by the above helper for computeKnownFPClass, so you shouldn't need the select here (and later).


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:4123
+      // instruction above
+      assert(!C->isNaN() && "Unexpected NaN  constant!");
       // TODO: We can catch more cases by using a range check rather than
----------------
Nit: extra space.


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

https://reviews.llvm.org/D151887