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

[Matt Arsenault via Phabricator via llvm-commits]

arsenm 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(
----------------
jcranmer-intel wrote:
> 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.
Another reason is the operand doesn't really need to be immarg. The AMDGPU class instruction handles a register just fine (in fact nearly every test mask needs to be materialized). In that case we would want folds to a canonicalizable constant value


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

https://reviews.llvm.org/D137811