[PATCH] D157270: [Clang][AArch64] Add diagnostic for calls from non-ZA to shared-ZA functions.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.


================
Comment at: clang/test/Sema/aarch64-sme-func-attrs.c:181
 
+void non_za_definition(void) {
+  sme_arm_new_za(); // OK
----------------
rsandifo-arm wrote:
> sdesmalen wrote:
> > rsandifo-arm wrote:
> > > Would be good to have some tests for indirect function calls too (via function pointers), to make sure that the diagnostic still works when no decl is available.
> > > 
> > > I suppose this applies to D157269 too.
> > I'm not sure that's necessary because D127762 already added tests to ensure the attributes propagate on pointer types, which then sets the ExtProtoInfo for those values. This patch merely checks the SME attribute fields from ExtProtoInfo. i.e. there is already nothing depending on a declaration being available.
> But `Sema::checkCall` does have some tests that depend on the decl rather than the type.  So the purpose of the test wouldn't be “does the attribute stick when applied to indirect function types?” (which I agree is already covered), but “does the new code correctly process the attribute on the target of a function pointer type?”
The declaration is only relevant for the call-site, not the callee.

  if (ExtInfo.AArch64SMEAttributes & FunctionType::SME_PStateZASharedMask) {

The above line checks __arm_shared_za attribute of the callee (could be a decl, or a function pointer, but in either case is a prototyped function with the propagated attributes)

  if (auto *CallerFD = dyn_cast<FunctionDecl>(CurContext)) {

The above line checks if the call-site context is a FunctionDecl (or definition for that matter). If the call is not part of a declaration (e.g. it's part of some global initialiser), we know it cannot have any live ZA state (which I now realise is missing a test-case).

So I think that a test like this:

  __arm_new_za void foo(void (*f)() __arm_shared_za) { f(); }

is not testing anything that isn't already tested. But perhaps I'm still misunderstanding your point. If so, could you give an example of a test you have in mind?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157270