[PATCH] D69281: [FPEnv][WIP] Constrained FCmp intrinsics

[Andy Kaylor via Phabricator via llvm-commits]

andrew.w.kaylor added inline comments.


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:750-791
+  def int_experimental_constrained_fcmpoeq
+      : Intrinsic<[ LLVMScalarOrSameVectorWidth<0, llvm_i1_ty> ],
+                  [ llvm_anyfloat_ty, LLVMMatchType<0>, llvm_metadata_ty ]>;
+  def int_experimental_constrained_fcmpogt
+      : Intrinsic<[ LLVMScalarOrSameVectorWidth<0, llvm_i1_ty> ],
+                  [ llvm_anyfloat_ty, LLVMMatchType<0>, llvm_metadata_ty ]>;
+  def int_experimental_constrained_fcmpoge
----------------
cameron.mcinally wrote:
> uweigand wrote:
> > simoll wrote:
> > > Out of curiosity: what is your motivation to have one intrinsic per fcmp predicate instead of, say, an `i8 immarg`? 
> > I didn't really make the decision, just took it over from the original patch.  But I agree it seems to make sense simply from a perspective of being explicit in the IR: if there's just a random numeric value, it would make the IR harder to read/write.
> It would be hard to read -- unless we wrote an IR decoding routine somewhere. That seems weird though.
This is why we used metadata strings for the constraint arguments. I'm still not entirely satisfied with that.  Would it be reasonable to make these arguments constants and add a custom printer and parser for these intrinsics? What about token arguments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69281