[PATCH] D58353: SystemZ: Add ImmArg to intrinsics
Matt Arsenault via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 18 11:27:55 PST 2019
arsenm added a comment.
In D58353#1401332 <https://reviews.llvm.org/D58353#1401332>, @uweigand wrote:
> In D58353#1401325 <https://reviews.llvm.org/D58353#1401325>, @arsenm wrote:
>
> > 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?
>
>
> Yes, they are ... but note that all the intrinsics I listed above are actually marked as requiring immediates there too. (E.g. int_s390_vfmaxdb gets emitted from __builtin_s390_vfmaxdb, which has an "Ii" marker.)
These ones aren't using GCCBuiltin, so that's why they were missed
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58353/new/
https://reviews.llvm.org/D58353
More information about the llvm-commits
mailing list