[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu May 5 06:18:29 PDT 2022
aaron.ballman added inline comments.
================
Comment at: clang/include/clang-c/Index.h:3448
+ CXCallingConv_AArch64SVEPcs= 17,
+ CXCallingConv_SwiftAsync = 18,
----------------
peterwaller-arm wrote:
> peterwaller-arm wrote:
> > It shouldn't matter in principle (... "but in practice" ...) we should probably avoid renumbering existing things in the enum and instead add to the end of it.
> >
> > Nit, this is missing a space before the equals.
> > Nit, SVE is an acronym, so is PCS, so capitalization should be consistent between the two. I see 'PCS' capitalized in AAPCS for example so probably all upper case makes the sense.
> >
> I retract my sloppy "it shouldn't matter in principle [at the source level]", of course it does matter, and it likely matters in this case (see 'alias for compatibility' comment above).
>
> To be more specific, changing the enum is an ABI break, and breaks if these things are ever serialized and therefore not something you want to do.
+1 -- I was just making that comment when you beat me to it.
================
Comment at: clang/include/clang/Basic/Specifiers.h:283
CC_AArch64VectorCall, // __attribute__((aarch64_vector_pcs))
+ CC_AArch64SVEPcs, // __attribute__((aarch64_sve_pcs))
};
----------------
I continue to be concerned with how many custom calling conventions we're adding. We're running out of bits to store them and when we bump up against the next limitation (32 possible calling conventions, this patch brings us up to 20), we're going to lose a significant amount of performance due to extra memory overhead pressure (which, in turn, will greatly reduce our maximum template instantiation depth) with no clear path forward. We've looked into this before and once we hit that 32 calling convention limit, we won't be adding any new calling conventions until we solve the memory pressure problem.
Are there other alternatives than making a new calling convention? (e.g., can this be handled with a backend pass somehow, or some other implementation strategy?)
Is this calling convention going to be used often enough to warrant adding it? (e.g., will this be showing up in system headers or other major third-party library headers, or is this a one-off for people trying to eek the last ounce of performance out of a function call?)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124998/new/
https://reviews.llvm.org/D124998
More information about the cfe-commits
mailing list