[PATCH] D137811: InstCombine: Port amdgcn.class intrinsic combines to is.fpclass

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:6058-6066
+    // If all tests are made, it doesn't matter what the value is.
+    if ((Mask & fcAllFlags) == fcAllFlags)
+      return ConstantInt::get(ReturnType, true);
+    if ((Mask & fcAllFlags) == 0)
+      return ConstantInt::get(ReturnType, false);
+    if (isa<PoisonValue>(Op0))
+      return PoisonValue::get(ReturnType);
----------------
arsenm wrote:
> foad wrote:
> > Poison/undef checks should probably go first, before other simplifications?
> I specifically had these later. The first operand shouldn't matter based on the test. I'm thinking about the interaction between fast math optimizations, and class uses used to guard regions where nnan/ninf is expected.
It's maybe not worth arguing about, but...

LangRef says "Most instructions return ‘poison’ when one of their arguments is ‘poison’". If you're saying is.fpclass is an exception to that rule then it at least ought to be documented. But I'm not sure why simplifying to poison would be a problem for the kind of code you're talking about - do you have an example?


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

https://reviews.llvm.org/D137811