[PATCH] D58353: SystemZ: Add ImmArg to intrinsics

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

In D58353#1401312 <https://reviews.llvm.org/D58353#1401312>, @uweigand wrote:

> 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.)


I'm doing this blindly based on the definitions here https://github.com/llvm-mirror/clang/blob/master/include/clang/Basic/BuiltinsSystemZ.def
Are these accurate and complete?


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

https://reviews.llvm.org/D58353