[PATCH] D54649: [FPEnv] Rough out constrained FCmp intrinsics

Mon Dec 17 11:34:26 PST 2018

andrew.w.kaylor added a comment.

I'm a bit out of my depth with the tablegen type handling stuff. I have some minor comments regarding naming consistency. Everything looks reasonable to me.

Perhaps it would be worth renaming this review to make clear that it is primarily an extension to the tablegen type handling and only implements part of the strict FCMP stuff in order to have a use for the new type.

================
Comment at: include/llvm/IR/Intrinsics.h:104
+      SameVecWidthArgument, PtrToArgument, PtrToElt, VecOfAnyPtrsToElt,
+      SameVecLengthOrScalarArgument
     } Kind;
----------------
This should probably be called SameVectorWidthOrScalarArgument for consistency.

================
Comment at: include/llvm/IR/Intrinsics.td:608
+  def int_experimental_constrained_fcmpeq : Intrinsic<[ llvm_anybool_ty ],
+                                                      [ LLVMVectorSameLengthOrScalar<0, llvm_anyfloat_ty>,
+                                                        LLVMVectorSameLengthOrScalar<0, llvm_anyfloat_ty>,
----------------
You have a problem with line wrap here.

================
Comment at: lib/IR/Function.cpp:693
+  IIT_F128 = 41,
+  IIT_SAME_VEC_LENGTH_OR_SCALAR_ARG = 42
 };
----------------
IIT_SAME_VEC_WIDTH_OR_SCALAR_ARG

Repository:
  rL LLVM

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

https://reviews.llvm.org/D54649