[PATCH] D58353: SystemZ: Add ImmArg to intrinsics

[Ulrich Weigand via Phabricator via llvm-commits]

uweigand added a comment.

I'm not quite sure I understand the logic why some intrinsics that require immediate arguments are marked with ImmArg, but others are not?

Shouldn't we mark all of them?   The ones I see missing in your patch are:

  defm int_s390_vfae  : SystemZTernaryIntCCBHF;
  defm int_s390_vfaez : SystemZTernaryIntCCBHF;
  defm int_s390_vstrc  : SystemZQuaternaryIntCCBHF;
  defm int_s390_vstrcz : SystemZQuaternaryIntCCBHF;
  def int_s390_vfmaxdb : Intrinsic<[llvm_v2f64_ty],
  def int_s390_vfmindb : Intrinsic<[llvm_v2f64_ty],
  def int_s390_vfmaxsb : Intrinsic<[llvm_v4f32_ty],
  def int_s390_vfminsb : Intrinsic<[llvm_v4f32_ty],
  def int_s390_vftcidb : SystemZBinaryConvIntCC<llvm_v2i64_ty, llvm_v2f64_ty>;
  def int_s390_vftcisb : SystemZBinaryConvIntCC<llvm_v4i32_ty, llvm_v4f32_ty>;
  def int_s390_vfidb : Intrinsic<[llvm_v2f64_ty],
  def int_s390_vfisb : Intrinsic<[llvm_v4f32_ty],

Maybe it would actually make more sense to add the ImmArg directly in the SystemZ*Int* helper macros; those intrinsics really all need immediate arguments.  (The sole exception I can see is verll, but that probably should use a different helper then.)


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

https://reviews.llvm.org/D58353