[PATCH] D157270: [Clang][AArch64] Add diagnostic for calls from non-ZA to shared-ZA functions.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sdesmalen marked 4 inline comments as done.
Closed by commit rG453c30e9e633: [Clang][AArch64] Add diagnostic for calls from
non-ZA to shared-ZA functions. (authored by sdesmalen).
Changed prior to commit:
https://reviews.llvm.org/D157270?vs=548131=548580#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157270/new/
https://reviews.llvm.org/D157270
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/CodeGen/aarch64-sme-intrinsics/aarch64-sme-attrs.cpp
clang/test/Sema/aarch64-sme-func-attrs.c
Index: clang/test/Sema/aarch64-sme-func-attrs.c
===
--- clang/test/Sema/aarch64-sme-func-attrs.c
+++ clang/test/Sema/aarch64-sme-func-attrs.c
@@ -178,7 +178,31 @@
void redecl_nopreserve_za(void) __arm_shared_za;
void redecl_nopreserve_za(void) __arm_shared_za __arm_preserves_za {}
+void non_za_definition(void (*shared_za_fn_ptr)(void) __arm_shared_za) {
+ sme_arm_new_za(); // OK
+ // expected-error@+2 {{call to a shared ZA function requires the caller to
have ZA state}}
+ // expected-cpp-error@+1 {{call to a shared ZA function requires the caller
to have ZA state}}
+ sme_arm_shared_za();
+ // expected-error@+2 {{call to a shared ZA function requires the caller to
have ZA state}}
+ // expected-cpp-error@+1 {{call to a shared ZA function requires the caller
to have ZA state}}
+ shared_za_fn_ptr();
+}
+
+void shared_za_definition(void (*shared_za_fn_ptr)(void) __arm_shared_za)
__arm_shared_za {
+ sme_arm_shared_za(); // OK
+ shared_za_fn_ptr(); // OK
+}
+
+__arm_new_za void new_za_definition(void (*shared_za_fn_ptr)(void)
__arm_shared_za) {
+ sme_arm_shared_za(); // OK
+ shared_za_fn_ptr(); // OK
+}
+
#ifdef __cplusplus
+int shared_za_initializer(void) __arm_shared_za;
+// expected-cpp-error@+1 {{call to a shared ZA function requires the caller to
have ZA state}}
+int global = shared_za_initializer();
+
struct S {
virtual void shared_za_memberfn(void) __arm_shared_za;
};
Index: clang/test/CodeGen/aarch64-sme-intrinsics/aarch64-sme-attrs.cpp
===
--- clang/test/CodeGen/aarch64-sme-intrinsics/aarch64-sme-attrs.cpp
+++ clang/test/CodeGen/aarch64-sme-intrinsics/aarch64-sme-attrs.cpp
@@ -255,7 +255,7 @@
template
__attribute__((always_inline))
-int call(T f, Other... other) {
+int call(T f, Other... other) __arm_shared_za {
return f() + call(other...);
}
@@ -270,7 +270,7 @@
// CHECK-NEXT: add nsw
// CHECK-NEXT: add nsw
// CHECK-NEXT: ret
-int test_variadic_template() {
+int test_variadic_template() __arm_shared_za {
return call(normal_callee,
streaming_decl,
streaming_compatible_decl,
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -6811,6 +6811,22 @@
Diag(Loc, diag::err_sme_call_in_non_sme_target);
}
}
+
+// If the callee uses AArch64 SME ZA state but the caller doesn't define
+// any, then this is an error.
+if (ExtInfo.AArch64SMEAttributes & FunctionType::SME_PStateZASharedMask) {
+ bool CallerHasZAState = false;
+ if (const auto *CallerFD = dyn_cast(CurContext)) {
+if (CallerFD->hasAttr())
+ CallerHasZAState = true;
+else if (const auto *FPT =
CallerFD->getType()->getAs())
+ CallerHasZAState = FPT->getExtProtoInfo().AArch64SMEAttributes &
+ FunctionType::SME_PStateZASharedMask;
+ }
+
+ if (!CallerHasZAState)
+Diag(Loc, diag::err_sme_za_call_no_za_state);
+}
}
if (FDecl && FDecl->hasAttr()) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3629,6 +3629,8 @@
"function declared %0 was previously declared %1, which has different SME
function attributes">;
def err_sme_call_in_non_sme_target : Error<
"call to a streaming function requires 'sme'">;
+def err_sme_za_call_no_za_state : Error<
+ "call to a shared ZA function requires the caller to have ZA state">;
def err_sme_definition_using_sm_in_non_sme_target : Error<
"function executed in streaming-SVE mode requires 'sme'">;
def err_sme_definition_using_za_in_non_sme_target : Error<
Index: clang/test/Sema/aarch64-sme-func-attrs.c
===
--- clang/test/Sema/aarch64-sme-func-attrs.c
+++ clang/test/Sema/aarch64-sme-func-attrs.c
@@ -178,7 +178,31 @@
void redecl_nopreserve_za(void) __arm_shared_za;
void
[PATCH] D157270: [Clang][AArch64] Add diagnostic for calls from non-ZA to shared-ZA functions.
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM aside from a nit.
Comment at: clang/lib/Sema/SemaChecking.cpp:6773
+ bool CallerHasZAState = false;
+ if (auto *CallerFD = dyn_cast(CurContext)) {
+if (CallerFD->hasAttr())
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157270/new/
https://reviews.llvm.org/D157270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157270: [Clang][AArch64] Add diagnostic for calls from non-ZA to shared-ZA functions.
sdesmalen updated this revision to Diff 548131.
sdesmalen added a comment.
Added test-cases for indirect calls
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157270/new/
https://reviews.llvm.org/D157270
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/aarch64-sme-func-attrs.c
Index: clang/test/Sema/aarch64-sme-func-attrs.c
===
--- clang/test/Sema/aarch64-sme-func-attrs.c
+++ clang/test/Sema/aarch64-sme-func-attrs.c
@@ -178,7 +178,31 @@
void redecl_nopreserve_za(void) __arm_shared_za;
void redecl_nopreserve_za(void) __arm_shared_za __arm_preserves_za {}
+void non_za_definition(void (*shared_za_fn_ptr)(void) __arm_shared_za) {
+ sme_arm_new_za(); // OK
+ // expected-error@+2 {{call to a shared ZA function requires the caller to
have ZA state}}
+ // expected-cpp-error@+1 {{call to a shared ZA function requires the caller
to have ZA state}}
+ sme_arm_shared_za();
+ // expected-error@+2 {{call to a shared ZA function requires the caller to
have ZA state}}
+ // expected-cpp-error@+1 {{call to a shared ZA function requires the caller
to have ZA state}}
+ shared_za_fn_ptr();
+}
+
+void shared_za_definition(void (*shared_za_fn_ptr)(void) __arm_shared_za)
__arm_shared_za {
+ sme_arm_shared_za(); // OK
+ shared_za_fn_ptr(); // OK
+}
+
+__arm_new_za void new_za_definition(void (*shared_za_fn_ptr)(void)
__arm_shared_za) {
+ sme_arm_shared_za(); // OK
+ shared_za_fn_ptr(); // OK
+}
+
#ifdef __cplusplus
+int shared_za_initializer(void) __arm_shared_za;
+// expected-cpp-error@+1 {{call to a shared ZA function requires the caller to
have ZA state}}
+int global = shared_za_initializer();
+
struct S {
virtual void shared_za_memberfn(void) __arm_shared_za;
};
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -6765,6 +6765,22 @@
Diag(Loc, diag::err_sme_call_in_non_sme_target);
}
}
+
+// If the callee uses AArch64 SME ZA state but the caller doesn't define
+// any, then this is an error.
+if (ExtInfo.AArch64SMEAttributes & FunctionType::SME_PStateZASharedMask) {
+ bool CallerHasZAState = false;
+ if (auto *CallerFD = dyn_cast(CurContext)) {
+if (CallerFD->hasAttr())
+ CallerHasZAState = true;
+else if (const auto *FPT =
CallerFD->getType()->getAs())
+ CallerHasZAState = FPT->getExtProtoInfo().AArch64SMEAttributes &
+ FunctionType::SME_PStateZASharedMask;
+ }
+
+ if (!CallerHasZAState)
+Diag(Loc, diag::err_sme_za_call_no_za_state);
+}
}
if (FDecl && FDecl->hasAttr()) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3629,6 +3629,8 @@
"function declared %0 was previously declared %1, which has different SME
function attributes">;
def err_sme_call_in_non_sme_target : Error<
"call to a streaming function requires 'sme'">;
+def err_sme_za_call_no_za_state : Error<
+ "call to a shared ZA function requires the caller to have ZA state">;
def err_sme_definition_using_sm_in_non_sme_target : Error<
"function executed in streaming-SVE mode requires 'sme'">;
def err_sme_definition_using_za_in_non_sme_target : Error<
Index: clang/test/Sema/aarch64-sme-func-attrs.c
===
--- clang/test/Sema/aarch64-sme-func-attrs.c
+++ clang/test/Sema/aarch64-sme-func-attrs.c
@@ -178,7 +178,31 @@
void redecl_nopreserve_za(void) __arm_shared_za;
void redecl_nopreserve_za(void) __arm_shared_za __arm_preserves_za {}
+void non_za_definition(void (*shared_za_fn_ptr)(void) __arm_shared_za) {
+ sme_arm_new_za(); // OK
+ // expected-error@+2 {{call to a shared ZA function requires the caller to have ZA state}}
+ // expected-cpp-error@+1 {{call to a shared ZA function requires the caller to have ZA state}}
+ sme_arm_shared_za();
+ // expected-error@+2 {{call to a shared ZA function requires the caller to have ZA state}}
+ // expected-cpp-error@+1 {{call to a shared ZA function requires the caller to have ZA state}}
+ shared_za_fn_ptr();
+}
+
+void shared_za_definition(void (*shared_za_fn_ptr)(void) __arm_shared_za) __arm_shared_za {
+ sme_arm_shared_za(); // OK
+ shared_za_fn_ptr(); // OK
+}
+
+__arm_new_za void new_za_definition(void (*shared_za_fn_ptr)(void) __arm_shared_za) {
+ sme_arm_shared_za(); // OK
+ shared_za_fn_ptr(); // OK
+}
+
#ifdef __cplusplus
+int shared_za_initializer(void) __arm_shared_za;
+// expected-cpp-error@+1 {{call to a shared ZA function requires the caller to have ZA state}}
+int global =
[PATCH] D157270: [Clang][AArch64] Add diagnostic for calls from non-ZA to shared-ZA functions.
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:
> > > 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(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?
> That's the kind of test I had in mind.
>
> checkCall does have some conditions that are based on the callee decl rather
> than the callee type. That is, it does distinguish between direct calls and
> indirect calls. It would have been easy for the code to be in a block that
> was guarded with FDecl. Or it could be accidentally rearranged like that in
> future, especially if the function grows to have several more tests.
>
> And yeah, I agree that the code as-written works, and so it doesn't fall into
> the trap that is being tested for. But that's true of most tests that get
> written for new features. :)
>
> So a test for both the direct and indirect cases seemed worthwhile. If we
> were just going to have one, then I think testing the indirect case is more
> valuable than testing the direct case, since it's the type rather than the
> decl that matters.
>
> I won't press the point further though. Feel free to skip the comment if
> you'd rather keep the tests as they are.
Thanks for elaborating, I was mostly trying to understand your reasoning for
adding this test.
> If we were just going to have one, then I think testing the indirect case is
> more valuable than testing the direct case, since it's the type rather than
> the decl that matters.
You're right that the tests should probably have used the indirect case from
the start. I see your argument about the code possibly changing in the future.
It seems unlikely that someone would add an extra FDecl guard above the block
where I've added the code, but I see how that's not really the point. I guess
there is little argument not to add another positive test-case to the patch, so
I'll just do that.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157270/new/
https://reviews.llvm.org/D157270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157270: [Clang][AArch64] Add diagnostic for calls from non-ZA to shared-ZA functions.
rsandifo-arm added inline comments.
Comment at: clang/test/Sema/aarch64-sme-func-attrs.c:181
+void non_za_definition(void) {
+ sme_arm_new_za(); // OK
sdesmalen wrote:
> 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(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?
That's the kind of test I had in mind.
checkCall does have some conditions that are based on the callee decl rather
than the callee type. That is, it does distinguish between direct calls and
indirect calls. It would have been easy for the code to be in a block that was
guarded with FDecl. Or it could be accidentally rearranged like that in
future, especially if the function grows to have several more tests.
And yeah, I agree that the code as-written works, and so it doesn't fall into
the trap that is being tested for. But that's true of most tests that get
written for new features. :)
So a test for both the direct and indirect cases seemed worthwhile. If we were
just going to have one, then I think testing the indirect case is more valuable
than testing the direct case, since it's the type rather than the decl that
matters.
I won't press the point further though. Feel free to skip the comment if you'd
rather keep the tests as they are.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157270/new/
https://reviews.llvm.org/D157270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157270: [Clang][AArch64] Add diagnostic for calls from non-ZA to shared-ZA functions.
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(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
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157270: [Clang][AArch64] Add diagnostic for calls from non-ZA to shared-ZA functions.
sdesmalen updated this revision to Diff 547936.
sdesmalen marked an inline comment as done.
sdesmalen added a comment.
- Replaced `|=` into normal assignment `=`
- Added test for global initializer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157270/new/
https://reviews.llvm.org/D157270
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/aarch64-sme-func-attrs.c
Index: clang/test/Sema/aarch64-sme-func-attrs.c
===
--- clang/test/Sema/aarch64-sme-func-attrs.c
+++ clang/test/Sema/aarch64-sme-func-attrs.c
@@ -178,7 +178,26 @@
void redecl_nopreserve_za(void) __arm_shared_za;
void redecl_nopreserve_za(void) __arm_shared_za __arm_preserves_za {}
+void non_za_definition(void) {
+ sme_arm_new_za(); // OK
+ // expected-error@+2 {{call to a shared ZA function requires the caller to
have ZA state}}
+ // expected-cpp-error@+1 {{call to a shared ZA function requires the caller
to have ZA state}}
+ sme_arm_shared_za();
+}
+
+void shared_za_definition(void) __arm_shared_za {
+ sme_arm_shared_za(); // OK
+}
+
+__arm_new_za void new_za_definition(void) {
+ sme_arm_shared_za(); // OK
+}
+
#ifdef __cplusplus
+int shared_za_initializer(void) __arm_shared_za;
+// expected-cpp-error@+1 {{call to a shared ZA function requires the caller to
have ZA state}}
+int global = shared_za_initializer();
+
struct S {
virtual void shared_za_memberfn(void) __arm_shared_za;
};
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -6765,6 +6765,22 @@
Diag(Loc, diag::err_sme_call_in_non_sme_target);
}
}
+
+// If the callee uses AArch64 SME ZA state but the caller doesn't define
+// any, then this is an error.
+if (ExtInfo.AArch64SMEAttributes & FunctionType::SME_PStateZASharedMask) {
+ bool CallerHasZAState = false;
+ if (auto *CallerFD = dyn_cast(CurContext)) {
+if (CallerFD->hasAttr())
+ CallerHasZAState = true;
+else if (const auto *FPT =
CallerFD->getType()->getAs())
+ CallerHasZAState = FPT->getExtProtoInfo().AArch64SMEAttributes &
+ FunctionType::SME_PStateZASharedMask;
+ }
+
+ if (!CallerHasZAState)
+Diag(Loc, diag::err_sme_za_call_no_za_state);
+}
}
if (FDecl && FDecl->hasAttr()) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3629,6 +3629,8 @@
"function declared %0 was previously declared %1, which has different SME
function attributes">;
def err_sme_call_in_non_sme_target : Error<
"call to a streaming function requires 'sme'">;
+def err_sme_za_call_no_za_state : Error<
+ "call to a shared ZA function requires the caller to have ZA state">;
def err_sme_definition_using_sm_in_non_sme_target : Error<
"function executed in streaming-SVE mode requires 'sme'">;
def err_sme_definition_using_za_in_non_sme_target : Error<
Index: clang/test/Sema/aarch64-sme-func-attrs.c
===
--- clang/test/Sema/aarch64-sme-func-attrs.c
+++ clang/test/Sema/aarch64-sme-func-attrs.c
@@ -178,7 +178,26 @@
void redecl_nopreserve_za(void) __arm_shared_za;
void redecl_nopreserve_za(void) __arm_shared_za __arm_preserves_za {}
+void non_za_definition(void) {
+ sme_arm_new_za(); // OK
+ // expected-error@+2 {{call to a shared ZA function requires the caller to have ZA state}}
+ // expected-cpp-error@+1 {{call to a shared ZA function requires the caller to have ZA state}}
+ sme_arm_shared_za();
+}
+
+void shared_za_definition(void) __arm_shared_za {
+ sme_arm_shared_za(); // OK
+}
+
+__arm_new_za void new_za_definition(void) {
+ sme_arm_shared_za(); // OK
+}
+
#ifdef __cplusplus
+int shared_za_initializer(void) __arm_shared_za;
+// expected-cpp-error@+1 {{call to a shared ZA function requires the caller to have ZA state}}
+int global = shared_za_initializer();
+
struct S {
virtual void shared_za_memberfn(void) __arm_shared_za;
};
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -6765,6 +6765,22 @@
Diag(Loc, diag::err_sme_call_in_non_sme_target);
}
}
+
+// If the callee uses AArch64 SME ZA state but the caller doesn't define
+// any, then this is an error.
+if (ExtInfo.AArch64SMEAttributes & FunctionType::SME_PStateZASharedMask) {
+ bool CallerHasZAState = false;
+ if (auto *CallerFD = dyn_cast(CurContext)) {
+if (CallerFD->hasAttr())
+ CallerHasZAState =
[PATCH] D157270: [Clang][AArch64] Add diagnostic for calls from non-ZA to shared-ZA functions.
rsandifo-arm added inline comments.
Comment at: clang/test/Sema/aarch64-sme-func-attrs.c:181
+void non_za_definition(void) {
+ sme_arm_new_za(); // OK
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?”
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157270/new/
https://reviews.llvm.org/D157270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157270: [Clang][AArch64] Add diagnostic for calls from non-ZA to shared-ZA functions.
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:
> 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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157270/new/
https://reviews.llvm.org/D157270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157270: [Clang][AArch64] Add diagnostic for calls from non-ZA to shared-ZA functions.
rsandifo-arm added inline comments.
Comment at: clang/lib/Sema/SemaChecking.cpp:6737
+else if (const auto *FPT =
CallerFD->getType()->getAs())
+ CallerHasZAState |= FPT->getExtProtoInfo().AArch64SMEAttributes &
+ FunctionType::SME_PStateZASharedMask;
Very minor, but it looked odd to me to have `|=` in this statement and `=`
above.
Comment at: clang/test/Sema/aarch64-sme-func-attrs.c:181
+void non_za_definition(void) {
+ sme_arm_new_za(); // OK
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157270/new/
https://reviews.llvm.org/D157270
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D157270: [Clang][AArch64] Add diagnostic for calls from non-ZA to shared-ZA functions.
sdesmalen created this revision.
sdesmalen added reviewers: rsandifo-arm, aaron.ballman.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
sdesmalen requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
The caller is required to have ZA state if it wants to share it with a callee.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D157270
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/aarch64-sme-func-attrs.c
Index: clang/test/Sema/aarch64-sme-func-attrs.c
===
--- clang/test/Sema/aarch64-sme-func-attrs.c
+++ clang/test/Sema/aarch64-sme-func-attrs.c
@@ -178,6 +178,21 @@
void redecl_nopreserve_za(void) __arm_shared_za;
void redecl_nopreserve_za(void) __arm_shared_za __arm_preserves_za {}
+void non_za_definition(void) {
+ sme_arm_new_za(); // OK
+ // expected-error@+2 {{call to a shared ZA function requires the caller to
have ZA state}}
+ // expected-cpp-error@+1 {{call to a shared ZA function requires the caller
to have ZA state}}
+ sme_arm_shared_za();
+}
+
+void shared_za_definition(void) __arm_shared_za {
+ sme_arm_shared_za(); // OK
+}
+
+__arm_new_za void new_za_definition(void) {
+ sme_arm_shared_za(); // OK
+}
+
#ifdef __cplusplus
struct S {
virtual void shared_za_memberfn(void) __arm_shared_za;
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -6725,6 +6725,22 @@
Diag(Loc, diag::err_sme_call_in_non_sme_target);
}
}
+
+// If the callee uses AArch64 SME ZA state but the caller doesn't define
+// any, then this is an error.
+if (ExtInfo.AArch64SMEAttributes & FunctionType::SME_PStateZASharedMask) {
+ bool CallerHasZAState = false;
+ if (auto *CallerFD = dyn_cast(CurContext)) {
+if (CallerFD->hasAttr())
+ CallerHasZAState = true;
+else if (const auto *FPT =
CallerFD->getType()->getAs())
+ CallerHasZAState |= FPT->getExtProtoInfo().AArch64SMEAttributes &
+ FunctionType::SME_PStateZASharedMask;
+ }
+
+ if (!CallerHasZAState)
+Diag(Loc, diag::err_sme_za_call_no_za_state);
+}
}
if (FDecl && FDecl->hasAttr()) {
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3626,6 +3626,8 @@
"function declared %0 was previously declared %1, which has different SME
function attributes">;
def err_sme_call_in_non_sme_target : Error<
"call to a streaming function requires sme">;
+def err_sme_za_call_no_za_state : Error<
+ "call to a shared ZA function requires the caller to have ZA state">;
def err_sme_definition_in_non_sme_target : Error<
"function executed in streaming-SVE mode or using ZA state, requires sme">;
def err_cconv_change : Error<
Index: clang/test/Sema/aarch64-sme-func-attrs.c
===
--- clang/test/Sema/aarch64-sme-func-attrs.c
+++ clang/test/Sema/aarch64-sme-func-attrs.c
@@ -178,6 +178,21 @@
void redecl_nopreserve_za(void) __arm_shared_za;
void redecl_nopreserve_za(void) __arm_shared_za __arm_preserves_za {}
+void non_za_definition(void) {
+ sme_arm_new_za(); // OK
+ // expected-error@+2 {{call to a shared ZA function requires the caller to have ZA state}}
+ // expected-cpp-error@+1 {{call to a shared ZA function requires the caller to have ZA state}}
+ sme_arm_shared_za();
+}
+
+void shared_za_definition(void) __arm_shared_za {
+ sme_arm_shared_za(); // OK
+}
+
+__arm_new_za void new_za_definition(void) {
+ sme_arm_shared_za(); // OK
+}
+
#ifdef __cplusplus
struct S {
virtual void shared_za_memberfn(void) __arm_shared_za;
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -6725,6 +6725,22 @@
Diag(Loc, diag::err_sme_call_in_non_sme_target);
}
}
+
+// If the callee uses AArch64 SME ZA state but the caller doesn't define
+// any, then this is an error.
+if (ExtInfo.AArch64SMEAttributes & FunctionType::SME_PStateZASharedMask) {
+ bool CallerHasZAState = false;
+ if (auto *CallerFD = dyn_cast(CurContext)) {
+if (CallerFD->hasAttr())
+ CallerHasZAState = true;
+else if (const auto *FPT = CallerFD->getType()->getAs())
+ CallerHasZAState |= FPT->getExtProtoInfo().AArch64SMEAttributes &
+ FunctionType::SME_PStateZASharedMask;
+ }
+
+ if (!CallerHasZAState)
+Diag(Loc, diag::err_sme_za_call_no_za_state);
