[PATCH] D124998: [AArch64][SVE] Add aarch64_sve_pcs attribute to Clang

Thu May 5 06:22:41 PDT 2022

tschuett added inline comments.

================
Comment at: clang/include/clang-c/Index.h:3448
+  CXCallingConv_AArch64SVEPcs= 17,
+  CXCallingConv_SwiftAsync = 18,

----------------
aaron.ballman wrote:
> 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.
Could you please add your CC as 18 and add a comment the next CC will 19 and do not touch the other ones.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124998