[PATCH] D138180: InstCombine: Fold negations of is_fpclass intrinsics

[Joshua Cranmer via Phabricator via llvm-commits]

jcranmer-intel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:3715-3717
+      II->setArgOperand(
+          1, ConstantInt::get(ClassMask->getType(),
+                              ~ClassMask->getZExtValue() & fcAllFlags));
----------------
sepavloff wrote:
> arsenm wrote:
> > sepavloff wrote:
> > > It must work for IEEE numbers. But for non-IEEE it is possible that a value does not belong to any of the classes known to `is_fpclass`. For example, type `x86_fp80` has so-called unsupported values. Do you think this transformation can be safely applied to such numbers also or it is better to limit it to IEEE numbers only?
> > I don't know anything about x87. How is the intrinsic lowered for it?
> > 
> > Is it just these "pseudo-X" cases? I'd assume those are covered by the non-pseudo tests?
> Yes, these are pseudo numbers. They are mapped to IEEE classes in more complex way, for example glibc recognizes pseudo-infinity as a NaN, and pseudo-denormals as normals.
> 
> Nevertheless any such number is mapped to one of IEEE classes and this optimization must work no matter how the mapping is realized.
>From what I can tell, all of the non-standard values for `x86_fp80` are mapped to returns true iff `is_fpclass` checks for both sNaN and qNaN, but not solely one.

I think `~is_fpclass(x, mask)` => `is_fpclass(x, ~mask)` is a valid transformation even for `x86_fp80`, even with this weird quirk.


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

https://reviews.llvm.org/D138180