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

[Cullen Rhodes via Phabricator via cfe-commits]

c-rhodes marked 2 inline comments as done.
c-rhodes added inline comments.


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


================
Comment at: clang/lib/CodeGen/CGCall.cpp:1361
+        Tmp.getAlignment().getAsAlign(),
+        llvm::ConstantInt::get(CGF.IntPtrTy, DstSize.getKnownMinSize()));
   }
----------------
@efriedma If we're happy with the element bitcast above this can also be fixed but I wasn't if that was ok, although it's pretty much what was implemented in the original codegen patch.


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

https://reviews.llvm.org/D85743