kmclaughlin accepted this revision.
kmclaughlin added a comment.
This revision is now accepted and ready to land.
Thank you for checking and removing EltTypeBool128. I think you have addressed
all of the other comments on this patch too, so it looks good to me!
Please can you update the commit message before landing, to change the
reference to `arm_sme_experimental.h` with
`arm_sme_draft_spec_subject_to_change.h`?
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:8874
case SVETypeFlags::EltTyBool64:
+ case SVETypeFlags::EltTyBool128:
return Builder.getInt1Ty();
----------------
bryanpkc wrote:
> kmclaughlin wrote:
> > Is it necessary to add an `EltTypeBool128`? I think the
> > EmitSVEPredicateCast call in EmitSMELd1St1 is creating a vector based on
> > the memory element type and not the predicate type?
> You are right, `EltTypeBool128` is not used right now; I can remove it. When
> we started working on the pre-ACLE intrinsics in our downstream compiler, we
> were planning to add something like `svptrue_b128`, even though the hardware
> `PTRUE` instruction doesn't support the Q size specifier. It is still not
> clear to me how the ACLE wants users to create a predicate vector for a call
> to a `_za128` intrinsic.
I think the reason why `svptrue_b128` wasn't added is because there is no
`ptrue.q` instruction as you say, and it is possible to use any of the other
forms (.b, .h, etc) with the 128 bit instructions since they only require every
16th lane.
It would be possible for the user to create a 128 bit predicate with either
svunpk or svzip of svbool_t with pfalse where necessary. Otherwise using the
other forms, e.g `svptrue_b64()`, will achieve the same result and require
fewer instructions.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127910/new/
https://reviews.llvm.org/D127910
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
