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

Thu May 5 13:10:51 PDT 2022

aaron.ballman added a comment.

In D124998#3494814 <https://reviews.llvm.org/D124998#3494814>, @erichkeane wrote:

> First: I care very little about `FunctionNoProtoType`.

I care insomuch as it doesn't break valid user code.

> Second, it is used only in "C" afaik, where our memory pressure isn't that big of a deal.  I'd be OK with having the `FunctionTypeExtraBits` be just added to `FunctionNoProtoType`, but stay in `FunctionProtoType` trailing storage.

I agree that the memory pressure here shouldn't be overly concerning, and I would most likely be okay if we adds the bits that way. However, I think we could also just make `FunctionNoProtoType` have the same memory layout as `FunctionProtoType` by using trailing objects again.

> It WOULD be more work, but I would wonder if we could combine all 3 classes into `FunctionType` and replace the `NoProtoType`-ness with a single bit to differentiate.  The result is the non-prototyped versions needing to carry around extra information, but since:
> a- our C implementation has low memory pressure
> b- prototypeless functions were a mistake
>
> I would think the extra memory they end up carrying might be ok?

We do a fair amount of checks via `isa<FunctionProtoType>()` (or similar) that would need to be updated to check for a `FunctionType` and whether that bit is set, and I don't have a good handle on what that would do to the code. If it doesn't cause significant maintenance issues, I would still want to see some measurements for what it does to compiler performance on large projects (it'd be a good thing to try on http://llvm-compile-time-tracker.com/).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124998