Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang][AArch64] Extend diagnostics when warning non/streaming about … #88380

Merged
merged 4 commits into from
Apr 24, 2024

Conversation

dtemirbulatov
Copy link
Contributor

…vector size difference

Add separate messages about passing arguments or returning parameters with scalable types.

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Apr 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 11, 2024

@llvm/pr-subscribers-clang

Author: Dinar Temirbulatov (dtemirbulatov)

Changes

…vector size difference

Add separate messages about passing arguments or returning parameters with scalable types.


Full diff: https://github.com/llvm/llvm-project/pull/88380.diff

5 Files Affected:

  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3-4)
  • (modified) clang/lib/Sema/SemaChecking.cpp (+13-5)
  • (modified) clang/lib/Sema/SemaDecl.cpp (+7-3)
  • (modified) clang/test/Sema/aarch64-incompat-sm-builtin-calls.c (+8-4)
  • (modified) clang/test/Sema/aarch64-sme-func-attrs.c (+24-24)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 059a8f58da5db1..7361400460b1cd 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3756,12 +3756,11 @@ def err_sme_definition_using_za_in_non_sme_target : Error<
 def err_sme_definition_using_zt0_in_non_sme2_target : Error<
   "function using ZT0 state requires 'sme2'">;
 def warn_sme_streaming_pass_return_vl_to_non_streaming : Warning<
-  "passing a VL-dependent argument to/from a function that has a different"
-  " streaming-mode. The streaming and non-streaming vector lengths may be"
-  " different">,
+  "%select{returning|passing}0 a VL-dependent argument %select{from|to}0 a function with a different"
+  " streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime">,
   InGroup<AArch64SMEAttributes>, DefaultIgnore;
 def warn_sme_locally_streaming_has_vl_args_returns : Warning<
-  "passing/returning a VL-dependent argument to/from a __arm_locally_streaming"
+  "%select{returning|passing}0 a VL-dependent argument %select{from|to}0 a __arm_locally_streaming"
   " function. The streaming and non-streaming vector"
   " lengths may be different">,
   InGroup<AArch64SMEAttributes>, DefaultIgnore;
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index abfd9a3031577b..f711bc8e9ca096 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -7938,7 +7938,8 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
     // For variadic functions, we may have more args than parameters.
     // For some K&R functions, we may have less args than parameters.
     const auto N = std::min<unsigned>(Proto->getNumParams(), Args.size());
-    bool AnyScalableArgsOrRet = Proto->getReturnType()->isSizelessVectorType();
+    bool IsScalableRet = Proto->getReturnType()->isSizelessVectorType();
+    bool IsScalableArg = false;
     for (unsigned ArgIdx = 0; ArgIdx < N; ++ArgIdx) {
       // Args[ArgIdx] can be null in malformed code.
       if (const Expr *Arg = Args[ArgIdx]) {
@@ -7953,7 +7954,7 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
 
         QualType ParamTy = Proto->getParamType(ArgIdx);
         if (ParamTy->isSizelessVectorType())
-          AnyScalableArgsOrRet = true;
+          IsScalableArg = true;
         QualType ArgTy = Arg->getType();
         CheckArgAlignment(Arg->getExprLoc(), FDecl, std::to_string(ArgIdx + 1),
                           ArgTy, ParamTy);
@@ -7978,7 +7979,8 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
     // arguments or return values, then warn the user that the streaming and
     // non-streaming vector lengths may be different.
     const auto *CallerFD = dyn_cast<FunctionDecl>(CurContext);
-    if (CallerFD && (!FD || !FD->getBuiltinID()) && AnyScalableArgsOrRet) {
+    if (CallerFD && (!FD || !FD->getBuiltinID()) &&
+        (IsScalableArg || IsScalableRet)) {
       bool IsCalleeStreaming =
           ExtInfo.AArch64SMEAttributes & FunctionType::SME_PStateSMEnabledMask;
       bool IsCalleeStreamingCompatible =
@@ -7987,8 +7989,14 @@ void Sema::checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
       ArmStreamingType CallerFnType = getArmStreamingFnType(CallerFD);
       if (!IsCalleeStreamingCompatible &&
           (CallerFnType == ArmStreamingCompatible ||
-           ((CallerFnType == ArmStreaming) ^ IsCalleeStreaming)))
-        Diag(Loc, diag::warn_sme_streaming_pass_return_vl_to_non_streaming);
+           ((CallerFnType == ArmStreaming) ^ IsCalleeStreaming))) {
+        if (IsScalableArg)
+          Diag(Loc, diag::warn_sme_streaming_pass_return_vl_to_non_streaming)
+              << /*IsArg=*/true;
+        if (IsScalableRet)
+          Diag(Loc, diag::warn_sme_streaming_pass_return_vl_to_non_streaming)
+              << /*IsArg=*/false;
+      }
     }
 
     FunctionType::ArmStateValue CalleeArmZAState =
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 5a23179dfbbf44..1ae3029df50b5c 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -12404,12 +12404,16 @@ bool Sema::CheckFunctionDeclaration(Scope *S, FunctionDecl *NewFD,
     bool UsesZT0 = Attr && Attr->isNewZT0();
 
     if (NewFD->hasAttr<ArmLocallyStreamingAttr>()) {
-      if (NewFD->getReturnType()->isSizelessVectorType() ||
-          llvm::any_of(NewFD->parameters(), [](ParmVarDecl *P) {
+      if (NewFD->getReturnType()->isSizelessVectorType())
+        Diag(NewFD->getLocation(),
+             diag::warn_sme_locally_streaming_has_vl_args_returns)
+            << /*IsArg=*/false;
+      if (llvm::any_of(NewFD->parameters(), [](ParmVarDecl *P) {
             return P->getOriginalType()->isSizelessVectorType();
           }))
         Diag(NewFD->getLocation(),
-             diag::warn_sme_locally_streaming_has_vl_args_returns);
+             diag::warn_sme_locally_streaming_has_vl_args_returns)
+            << /*IsArg=*/true;
     }
     if (const auto *FPT = NewFD->getType()->getAs<FunctionProtoType>()) {
       FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
diff --git a/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c b/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c
index 6a1feeb9bf5397..f5b770e17b1210 100644
--- a/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c
+++ b/clang/test/Sema/aarch64-incompat-sm-builtin-calls.c
@@ -33,7 +33,8 @@ svuint32_t incompat_sve_sm(svbool_t pg, svuint32_t a, int16_t b) __arm_streaming
   return __builtin_sve_svld1_gather_u32base_index_u32(pg, a, b);
 }
 
-// expected-warning@+1 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
+// expected-warning@+2 {{returning a VL-dependent argument from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
+// expected-warning@+1 {{passing a VL-dependent argument to a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
 __arm_locally_streaming svuint32_t incompat_sve_ls(svbool_t pg, svuint32_t a, int64_t b) {
   // expected-warning@+1 {{builtin call has undefined behaviour when called from a streaming function}}
   return __builtin_sve_svld1_gather_u32base_index_u32(pg, a, b);
@@ -49,7 +50,8 @@ svuint32_t incompat_sve2_sm(svbool_t pg, svuint32_t a, int64_t b) __arm_streamin
   return __builtin_sve_svldnt1_gather_u32base_index_u32(pg, a, b);
 }
 
-// expected-warning@+1 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
+// expected-warning@+2 {{returning a VL-dependent argument from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
+// expected-warning@+1 {{passing a VL-dependent argument to a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
 __arm_locally_streaming svuint32_t incompat_sve2_ls(svbool_t pg, svuint32_t a, int64_t b) {
   // expected-warning@+1 {{builtin call has undefined behaviour when called from a streaming function}}
   return __builtin_sve_svldnt1_gather_u32base_index_u32(pg, a, b);
@@ -70,7 +72,8 @@ svfloat64_t streaming_caller_sve(svbool_t pg, svfloat64_t a, float64_t b) __arm_
   return svadd_n_f64_m(pg, a, b);
 }
 
-// expected-warning@+1 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
+// expected-warning@+2 {{returning a VL-dependent argument from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
+// expected-warning@+1 {{passing a VL-dependent argument to a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
 __arm_locally_streaming svfloat64_t locally_streaming_caller_sve(svbool_t pg, svfloat64_t a, float64_t b) {
   // expected-no-warning
   return svadd_n_f64_m(pg, a, b);
@@ -86,7 +89,8 @@ svint16_t streaming_caller_sve2(svint16_t op1, svint16_t op2) __arm_streaming {
   return svmul_lane_s16(op1, op2, 0);
 }
 
-// expected-warning@+1 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
+// expected-warning@+2 {{returning a VL-dependent argument from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
+// expected-warning@+1 {{passing a VL-dependent argument to a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
 __arm_locally_streaming svint16_t locally_streaming_caller_sve2(svint16_t op1, svint16_t op2) {
   // expected-no-warning
   return svmul_lane_s16(op1, op2, 0);
diff --git a/clang/test/Sema/aarch64-sme-func-attrs.c b/clang/test/Sema/aarch64-sme-func-attrs.c
index 12de16509ccb8d..74ab02aff2288e 100644
--- a/clang/test/Sema/aarch64-sme-func-attrs.c
+++ b/clang/test/Sema/aarch64-sme-func-attrs.c
@@ -509,73 +509,73 @@ void sme_no_streaming_with_vl_arg(__SVInt8_t a) { }
 
 __SVInt8_t sme_no_streaming_returns_vl(void) { __SVInt8_t r; return r; }
 
-// expected-warning@+2 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
-// expected-cpp-warning@+1 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
+// expected-warning@+2 {{passing a VL-dependent argument to a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
+// expected-cpp-warning@+1 {{passing a VL-dependent argument to a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
 __arm_locally_streaming void sme_locally_streaming_with_vl_arg(__SVInt8_t a) { }
 
-// expected-warning@+2 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
-// expected-cpp-warning@+1 {{passing/returning a VL-dependent argument to/from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
+// expected-warning@+2 {{returning a VL-dependent argument from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
+// expected-cpp-warning@+1 {{returning a VL-dependent argument from a __arm_locally_streaming function. The streaming and non-streaming vector lengths may be different}}
 __arm_locally_streaming __SVInt8_t sme_locally_streaming_returns_vl(void) { __SVInt8_t r; return r; }
 
 void sme_no_streaming_calling_streaming_with_vl_args() {
   __SVInt8_t a;
-  // expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
-  // expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
+  // expected-warning@+2 {{passing a VL-dependent argument to a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
+  // expected-cpp-warning@+1 {{passing a VL-dependent argument to a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
   sme_streaming_with_vl_arg(a);
 }
 
 void sme_no_streaming_calling_streaming_with_return_vl() {
-  // expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
-  // expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
+  // expected-warning@+2 {{returning a VL-dependent argument from a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
+  // expected-cpp-warning@+1 {{returning a VL-dependent argument from a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
   __SVInt8_t r = sme_streaming_returns_vl();
 }
 
 void sme_streaming_calling_non_streaming_with_vl_args(void) __arm_streaming {
   __SVInt8_t a;
-  // expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
-  // expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
+  // expected-warning@+2 {{passing a VL-dependent argument to a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
+  // expected-cpp-warning@+1 {{passing a VL-dependent argument to a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
   sme_no_streaming_with_vl_arg(a);
 }
 
 void sme_streaming_calling_non_streaming_with_return_vl(void) __arm_streaming {
-  // expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
-  // expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
+  // expected-warning@+2 {{returning a VL-dependent argument from a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
+  // expected-cpp-warning@+1 {{returning a VL-dependent argument from a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
   __SVInt8_t r = sme_no_streaming_returns_vl();
 }
 
 void sme_no_streaming_calling_streaming_with_vl_args_param(__SVInt8_t arg, void (*sc)( __SVInt8_t arg) __arm_streaming) {
-  // expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
-  // expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
+  // expected-warning@+2 {{passing a VL-dependent argument to a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
+  // expected-cpp-warning@+1 {{passing a VL-dependent argument to a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
   sc(arg);
 }
 
 __SVInt8_t sme_no_streaming_calling_streaming_return_vl_param(__SVInt8_t (*s)(void) __arm_streaming) {
-  // expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
-  // expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
+  // expected-warning@+2 {{returning a VL-dependent argument from a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
+  // expected-cpp-warning@+1 {{returning a VL-dependent argument from a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
   return s();
 }
 
 void sme_streaming_compatible_calling_streaming_with_vl_args(__SVInt8_t arg) __arm_streaming_compatible {
-  // expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
-  // expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
+  // expected-warning@+2 {{passing a VL-dependent argument to a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
+  // expected-cpp-warning@+1 {{passing a VL-dependent argument to a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
   sme_streaming_with_vl_arg(arg);
 }
 
 void sme_streaming_compatible_calling_sme_streaming_return_vl(void) __arm_streaming_compatible {
-  // expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
-  // expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
+  // expected-warning@+2 {{returning a VL-dependent argument from a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
+  // expected-cpp-warning@+1 {{returning a VL-dependent argument from a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
   __SVInt8_t r = sme_streaming_returns_vl();
 }
 
 void sme_streaming_compatible_calling_no_streaming_with_vl_args(__SVInt8_t arg) __arm_streaming_compatible {
-  // expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
-  // expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
+  // expected-warning@+2 {{passing a VL-dependent argument to a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
+  // expected-cpp-warning@+1 {{passing a VL-dependent argument to a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
   sme_no_streaming_with_vl_arg(arg);
 }
 
 void sme_streaming_compatible_calling_no_sme_streaming_return_vl(void) __arm_streaming_compatible {
-  // expected-warning@+2 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
-  // expected-cpp-warning@+1 {{passing a VL-dependent argument to/from a function that has a different streaming-mode. The streaming and non-streaming vector lengths may be different}}
+  // expected-warning@+2 {{returning a VL-dependent argument from a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
+  // expected-cpp-warning@+1 {{returning a VL-dependent argument from a function with a different streaming-mode is undefined behaviour if the streaming and non-streaming vector lengths are different at runtime}}
   __SVInt8_t r = sme_no_streaming_returns_vl();
 }
 

Copy link
Collaborator

@sdesmalen-arm sdesmalen-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left two nits, but otherwise looks good.

InGroup<AArch64SMEAttributes>, DefaultIgnore;
def warn_sme_locally_streaming_has_vl_args_returns : Warning<
"passing/returning a VL-dependent argument to/from a __arm_locally_streaming"
"%select{returning|passing}0 a VL-dependent argument %select{from|to}0 a __arm_locally_streaming"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to have a similar message here, as you did above, e.g.

returning a VL-dependent argument from a locally streaming function is undefined behaviour when the streaming and non-streaming vector lengths are different at runtime

and

passing a VL-dependent argument to a locally streaming function is undefined behaviour when the streaming and non-streaming vector lengths are different at runtime

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved
dtemirbulatov and others added 4 commits April 24, 2024 09:38
…vector size difference

Add separate messages about passing arguments or returning parameters with scalable types.
Co-authored-by: Sander de Smalen <sander.desmalen@arm.com>
@dtemirbulatov dtemirbulatov merged commit bd34bc6 into llvm:main Apr 24, 2024
2 of 4 checks passed
@dtemirbulatov dtemirbulatov deleted the sme-warn-call-adjust branch April 24, 2024 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants