[PATCH] D85743: [CodeGen][AArch64] Support arm_sve_vector_bits attribute

Eli Friedman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 14 14:54:16 PDT 2020


efriedma accepted this revision.
efriedma added a comment.
This revision is now accepted and ready to land.

LGTM

Like I mentioned on the review for the prototype, I still think we should try to implement a scheme that makes CK_BItCast between fixed and scalable types trivial.  Doing coercion this way is going to have a significant performance cost.  But there isn't any user-visible effect, so I'm fine with leaving that for a followup.



================
Comment at: clang/lib/AST/ItaniumMangle.cpp:3330
+// appendices to the Procedure Call Standard for the Arm Architecture, see:
+// https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#appendix-c-mangling
+void CXXNameMangler::mangleAArch64FixedSveVectorType(const VectorType *T) {
----------------
c-rhodes wrote:
> efriedma wrote:
> > Mangling them the same way is going to cause practical issues; they're different types from a C++ perspective, so they need distinct manglings.  For example, you'll crash the compiler if you refer to both foo<svint64_t> and foo<fixed_int64_t>.
> > Mangling them the same way is going to cause practical issues; they're different types from a C++ perspective, so they need distinct manglings. For example, you'll crash the compiler if you refer to both foo<svint64_t> and foo<fixed_int64_t>.
> 
> The ACLE is yet to define the mangling scheme for fixed-length SVE types so I kept the mangling the same, which is also what GCC currently does. After speaking with @rsandifo-arm yesterday we agreed to come up with a mangling scheme where the types are mangled in the same way as:
> 
> ```__SVE_VLS<typename, unsigned>```
> 
> where the first argument is the underlying variable-length type and the second argument is the SVE vector length in bits.  For example:
> 
> ```#if __ARM_FEATURE_SVE_BITS==512                    
> // Mangled as 9__SVE_VLSIu11__SVInt32_tLj512EE                                
> typedef svint32_t vec __attribute__((arm_sve_vector_bits(512)));              
> // Mangled as 9__SVE_VLSIu10__SVBool_tLj512EE                                  
> typedef svbool_t pred __attribute__((arm_sve_vector_bits(512)));              
> #endif```
> 
> let us know if you have any feedback/concerns about this approach.
Makes sense.


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

https://reviews.llvm.org/D85743



More information about the cfe-commits mailing list