[PATCH] D54121: [FPEnv] Add constrained FCMP intrinsic

[Cameron McInally via Phabricator via llvm-commits]

cameron.mcinally added a comment.

That's cool with me. Definitely a ton of work, but it's clean and complete.

If I'm not mistaken, we could use inverse operations and swap the true/false operands for **SOME** of these. I'll have to dig through IEEE-754 to confirm which ones, but it could save some typing. Although, the exact relations are fairly dense, so front end writers won't be happy. E.g.:

  compareSignalingGreaterEqual(a,b) is equivalent to compareSignalingLessUnordered(b, a)

My opinion is not very strong on this though. The brevity might not be worth the added complexity. At least, not as a first pass at these operations.

Also, I see you added llvm_anyi1_ty. So we would have to extend TableGen to handle that, I assume. And we'd also have to extend TableGen to handle the LLVMScalarOrSameVectorWidth<0, llvm_anyfloat_ty> problem. That should really be "llvm_anyscalarfloat_ty" or something like that.

And finally nit-picking a little here, but this should really be:

  def int_experimental_constrained_fcmp_eq_quiet : Intrinsic<[ llvm_anyi1_ty ],
                                                             [ LLVMScalarOrSameVectorWidth<0, llvm_anyfloat_ty>,
                                                               LLVMMatchType<0>,
                                                               llvm_metadata_ty,
                                                               llvm_metadata_ty ]>;


Repository:
  rL LLVM

https://reviews.llvm.org/D54121