[PATCH] D139130: InstCombine: Fold and/or of fcmp into class

[Jay Foad via Phabricator via llvm-commits]

foad added subscribers: nikic, craig.topper.
foad added a comment.

Looks pretty clean to me overall.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1243
+      return {LHS, fcZero};
+    case FCmpInst::FCMP_UEQ: // match !(x != 0.0)
+      return {LHS, fcZero | fcNan};
----------------
Using `!=` in the comment is misleading since that is true for nans. Maybe use `<>` instead or just spell out what you mean.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1276
+    else if (Pred == FCmpInst::FCMP_UNE)
+      Class = fcNan | (~InfMask & fcAllFlags);
+    else if (Pred == FCmpInst::FCMP_ONE)
----------------
`fcNan |` is redundant here.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1351
+  // single fcmp with fneg and fabs, that's likely a better canonical form.
+  if (LHS->hasOneUse() && RHS->hasOneUse()) {
+    auto [ClassValLHS, ClassMaskLHS] = fcmpToClassTest(PredL, LHS0, LHS1);
----------------
Do you need to worry about `IsLogicalSelect` here? (@nikic, @craig.topper?)


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

https://reviews.llvm.org/D139130