[PATCH] D139130: InstCombine: Fold and (fcmp), (is.fpclass) into is.fpclass

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:829-843
+  if (Mask == fcNan && !IsStrict) {
+    // Equivalent of isnan. Replace with standard fcmp if we don't care about FP
+    // exceptions.
+    Value *FCmp = Builder.CreateIsNaN(Src0);
+    FCmp->takeName(&II);
+    return replaceInstUsesWith(II, FCmp);
+  }
----------------
arsenm wrote:
> sepavloff wrote:
> > Replacement of `llvm.is_fpclass` with comparison hides class checks and make analysis of code passes harder. Does it have any benefit over creation of the comparison in selector?
> Using fcmp makes analysis easier. fcmp has fewer constraints, and will always be better understood by generic passes than a special case intrinsic. It's a better canonical form; otherwise every single place that considers compares would have to perform the exact same checks for a class that performs a compare.
> 
> I also just noticed when I rebased I attached the wrong patch here.
fcmp also supports fast math flags, class does not


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

https://reviews.llvm.org/D139130