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

[Joshua Cranmer via Phabricator via llvm-commits]

jcranmer-intel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:856
+  // Clamp mask to used bits
+  if ((Mask & fcAllFlags) != Mask) {
+    CallInst *NewCall = Builder.CreateCall(
----------------
arsenm wrote:
> jyknight wrote:
> > Why are unknown bits even accepted? ISTM it should be an error in Verifier::visitIntrinsicCall to pass invalid bits.
> I don't know. Really this should be an i10 argument. I've been debating whether to add a verifier check, or make it an i10. I'd prefer to do that separately and clean up the bits here for now.
The LangRef documentation of `llvm.is.fpclass` doesn't pin down the handling for noncanonical values well. It's plausible they could be handled by extension of extra bits, but existing code seems to ignore them for `ppc_fp128` and treat them as NaNs that are neither qNaN nor sNaN for `x86_fp80`.

Not that it makes any difference to this patch, but it suggests that making it a verifier check instead of an `i10` is the better path, as it is slightly better future-proofed.


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

https://reviews.llvm.org/D137811