[clang] [Clang] Demote always_inline error to warning for mismatching SME attrs (PR #100740)

[Sander de Smalen via cfe-commits]

https://github.com/sdesmalen-arm updated https://github.com/llvm/llvm-project/pull/100740

>From 596654157c6e1124d5eac9fd415d957179897b90 Mon Sep 17 00:00:00 2001
From: Sander de Smalen <sander.desmalen at arm.com>
Date: Fri, 26 Jul 2024 13:24:10 +0100
Subject: [PATCH 1/2] [Clang] Demote always_inline error to warning for
 mismatching SME attrs

PR #77936 introduced a diagnostic to avoid calls being inlined into
functions with a different streaming mode, because inlining those
functions may result in different runtime behaviour. This was necessary
because LLVM doesn't check whether inlining is possible and thus blindly
inlines the function without checking feasibility.

In practice however, this introduces an artificial restriction that the
user may not be able to work around. Calling an `always_inline` function
from some header file that is out of the control of the user would
result in an error that the user cannot remedy.

Therefore, this patch demotes the error into a warning (for calls from
streaming[-compatible] -> non-streaming), but the proper fix would be to
fix the AlwaysInliner in LLVM to avoid inlining when it has analyzed the
callee and has determined that inlining is not possible.

Calling an always_inline function for calls from non-streaming ->
streaming will remain an error, because there is little pre-existing
code for SME, so it is expected that the header file can be modified by
the user (e.g. by using `__arm_streaming_compatible` if the code is
claimed to be compatible).
---
 clang/include/clang/Basic/DiagnosticFrontendKinds.td    | 3 +++
 clang/lib/CodeGen/Targets/AArch64.cpp                   | 6 ++++--
 clang/test/CodeGen/aarch64-sme-inline-streaming-attrs.c | 6 +++---
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 12a4617c64d87..04f87f5217067 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -288,6 +288,9 @@ def err_function_needs_feature : Error<
 let CategoryName = "Codegen ABI Check" in {
 def err_function_always_inline_attribute_mismatch : Error<
   "always_inline function %1 and its caller %0 have mismatching %2 attributes">;
+def warn_function_always_inline_attribute_mismatch : Warning<
+  "always_inline function %1 and its caller %0 have mismatching %2 attributes, "
+  "inlining may result in different runtime behaviour">, InGroup<AArch64SMEAttributes>;
 def err_function_always_inline_new_za : Error<
   "always_inline function %0 has new za state">;
 
diff --git a/clang/lib/CodeGen/Targets/AArch64.cpp b/clang/lib/CodeGen/Targets/AArch64.cpp
index b9df54b0c67c4..1dec3cd40ebd2 100644
--- a/clang/lib/CodeGen/Targets/AArch64.cpp
+++ b/clang/lib/CodeGen/Targets/AArch64.cpp
@@ -883,8 +883,10 @@ void AArch64TargetCodeGenInfo::checkFunctionCallABIStreaming(
 
   if (!CalleeIsStreamingCompatible &&
       (CallerIsStreaming != CalleeIsStreaming || CallerIsStreamingCompatible))
-    CGM.getDiags().Report(CallLoc,
-                          diag::err_function_always_inline_attribute_mismatch)
+    CGM.getDiags().Report(
+        CallLoc, CalleeIsStreaming
+                     ? diag::err_function_always_inline_attribute_mismatch
+                     : diag::warn_function_always_inline_attribute_mismatch)
         << Caller->getDeclName() << Callee->getDeclName() << "streaming";
   if (auto *NewAttr = Callee->getAttr<ArmNewAttr>())
     if (NewAttr->isNewZA())
diff --git a/clang/test/CodeGen/aarch64-sme-inline-streaming-attrs.c b/clang/test/CodeGen/aarch64-sme-inline-streaming-attrs.c
index 25aebeced9379..2c565f6225468 100644
--- a/clang/test/CodeGen/aarch64-sme-inline-streaming-attrs.c
+++ b/clang/test/CodeGen/aarch64-sme-inline-streaming-attrs.c
@@ -20,7 +20,7 @@ void caller(void) {
 
 #ifdef TEST_COMPATIBLE
 void caller_compatible(void) __arm_streaming_compatible {
-    inlined_fn(); // expected-error {{always_inline function 'inlined_fn' and its caller 'caller_compatible' have mismatching streaming attributes}}
+    inlined_fn(); // expected-warning {{always_inline function 'inlined_fn' and its caller 'caller_compatible' have mismatching streaming attributes, inlining may result in different runtime behaviour}}
     inlined_fn_streaming_compatible();
     inlined_fn_streaming(); // expected-error {{always_inline function 'inlined_fn_streaming' and its caller 'caller_compatible' have mismatching streaming attributes}}
     inlined_fn_local(); // expected-error {{always_inline function 'inlined_fn_local' and its caller 'caller_compatible' have mismatching streaming attributes}}
@@ -29,7 +29,7 @@ void caller_compatible(void) __arm_streaming_compatible {
 
 #ifdef TEST_STREAMING
 void caller_streaming(void) __arm_streaming {
-    inlined_fn(); // expected-error {{always_inline function 'inlined_fn' and its caller 'caller_streaming' have mismatching streaming attributes}}
+    inlined_fn(); // expected-warning {{always_inline function 'inlined_fn' and its caller 'caller_streaming' have mismatching streaming attributes, inlining may result in different runtime behaviour}}
     inlined_fn_streaming_compatible();
     inlined_fn_streaming();
     inlined_fn_local();
@@ -39,7 +39,7 @@ void caller_streaming(void) __arm_streaming {
 #ifdef TEST_LOCALLY
 __arm_locally_streaming
 void caller_local(void) {
-    inlined_fn(); // expected-error {{always_inline function 'inlined_fn' and its caller 'caller_local' have mismatching streaming attributes}}
+    inlined_fn(); // expected-warning {{always_inline function 'inlined_fn' and its caller 'caller_local' have mismatching streaming attributes, inlining may result in different runtime behaviour}}
     inlined_fn_streaming_compatible();
     inlined_fn_streaming();
     inlined_fn_local();

>From 377b19c69645e7ca525f61b14dbf17bbdbee3df1 Mon Sep 17 00:00:00 2001
From: Sander de Smalen <sander.desmalen at arm.com>
Date: Fri, 26 Jul 2024 15:11:24 +0100
Subject: [PATCH 2/2] Rephrase warning

---
 clang/include/clang/Basic/DiagnosticFrontendKinds.td    | 2 +-
 clang/test/CodeGen/aarch64-sme-inline-streaming-attrs.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
index 04f87f5217067..8a1462c670d68 100644
--- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -290,7 +290,7 @@ def err_function_always_inline_attribute_mismatch : Error<
   "always_inline function %1 and its caller %0 have mismatching %2 attributes">;
 def warn_function_always_inline_attribute_mismatch : Warning<
   "always_inline function %1 and its caller %0 have mismatching %2 attributes, "
-  "inlining may result in different runtime behaviour">, InGroup<AArch64SMEAttributes>;
+  "inlining may change runtime behaviour">, InGroup<AArch64SMEAttributes>;
 def err_function_always_inline_new_za : Error<
   "always_inline function %0 has new za state">;
 
diff --git a/clang/test/CodeGen/aarch64-sme-inline-streaming-attrs.c b/clang/test/CodeGen/aarch64-sme-inline-streaming-attrs.c
index 2c565f6225468..98370a888ac6c 100644
--- a/clang/test/CodeGen/aarch64-sme-inline-streaming-attrs.c
+++ b/clang/test/CodeGen/aarch64-sme-inline-streaming-attrs.c
@@ -20,7 +20,7 @@ void caller(void) {
 
 #ifdef TEST_COMPATIBLE
 void caller_compatible(void) __arm_streaming_compatible {
-    inlined_fn(); // expected-warning {{always_inline function 'inlined_fn' and its caller 'caller_compatible' have mismatching streaming attributes, inlining may result in different runtime behaviour}}
+    inlined_fn(); // expected-warning {{always_inline function 'inlined_fn' and its caller 'caller_compatible' have mismatching streaming attributes, inlining may change runtime behaviour}}
     inlined_fn_streaming_compatible();
     inlined_fn_streaming(); // expected-error {{always_inline function 'inlined_fn_streaming' and its caller 'caller_compatible' have mismatching streaming attributes}}
     inlined_fn_local(); // expected-error {{always_inline function 'inlined_fn_local' and its caller 'caller_compatible' have mismatching streaming attributes}}
@@ -29,7 +29,7 @@ void caller_compatible(void) __arm_streaming_compatible {
 
 #ifdef TEST_STREAMING
 void caller_streaming(void) __arm_streaming {
-    inlined_fn(); // expected-warning {{always_inline function 'inlined_fn' and its caller 'caller_streaming' have mismatching streaming attributes, inlining may result in different runtime behaviour}}
+    inlined_fn(); // expected-warning {{always_inline function 'inlined_fn' and its caller 'caller_streaming' have mismatching streaming attributes, inlining may change runtime behaviour}}
     inlined_fn_streaming_compatible();
     inlined_fn_streaming();
     inlined_fn_local();
@@ -39,7 +39,7 @@ void caller_streaming(void) __arm_streaming {
 #ifdef TEST_LOCALLY
 __arm_locally_streaming
 void caller_local(void) {
-    inlined_fn(); // expected-warning {{always_inline function 'inlined_fn' and its caller 'caller_local' have mismatching streaming attributes, inlining may result in different runtime behaviour}}
+    inlined_fn(); // expected-warning {{always_inline function 'inlined_fn' and its caller 'caller_local' have mismatching streaming attributes, inlining may change runtime behaviour}}
     inlined_fn_streaming_compatible();
     inlined_fn_streaming();
     inlined_fn_local();