[llvm-branch-commits] [clang] [clang][FMV][AArch64] Improve streaming mode compatibility (PR #101007)

Tobias Hieta via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Thu Aug 1 00:02:51 PDT 2024


https://github.com/tru updated https://github.com/llvm/llvm-project/pull/101007

>From b9203b6067c868d4305400f0964dbac8e15285db Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Tue, 23 Jul 2024 19:24:41 +0100
Subject: [PATCH 1/5] [clang][FMV][AArch64] Improve streaming mode
 compatibility.

* Allow arm-streaming if all the functions versions adhere to it.
* Allow arm-streaming-compatible if all the functions versions adhere to it.
* Allow arm-locally-streaming regardless of the other functions versions.

When the caller needs to toggle the streaming mode all the function versions
of the callee must adhere to the same mode, otherwise the call will yield a
runtime error.

Imagine the versions of the callee live in separate TUs. The version that
is visible to the caller will determine the calling convention used when
generating code for the callsite. Therefore we cannot support mixing
streaming with non-streaming function versions. Imagine TU1 has a streaming
caller and calls foo._sme which is streaming-compatible. The codegen for
the callsite will not switch off the streaming mode. Then in TU2 we have
a version which is non-streaming and could potentially be called in
streaming mode. Similarly if the caller is non-streaming and the called
version is streaming-compatible the codegen for the callsite will not
switch on the streaming mode, but other versions may be streaming.
---
 clang/include/clang/AST/ASTContext.h          | 16 +++++++
 .../clang/Basic/DiagnosticSemaKinds.td        |  2 -
 clang/lib/AST/ASTContext.cpp                  |  3 +-
 clang/lib/Sema/SemaDecl.cpp                   | 27 ++++++++++--
 clang/lib/Sema/SemaDeclAttr.cpp               |  7 ---
 clang/test/Sema/aarch64-fmv-streaming.c       | 43 +++++++++++++++++++
 clang/test/Sema/aarch64-sme-func-attrs.c      | 42 ------------------
 7 files changed, 83 insertions(+), 57 deletions(-)
 create mode 100644 clang/test/Sema/aarch64-fmv-streaming.c

diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 6d1c8ca8a2f96..a86394a51db16 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -3189,6 +3189,22 @@ class ASTContext : public RefCountedBase<ASTContext> {
       const FunctionDecl *FD,
       llvm::function_ref<void(FunctionDecl *)> Pred) const;
 
+  bool areFMVCompatible(const FunctionDecl *FD1,
+                        const FunctionDecl *FD2) const {
+    if (!hasSameType(FD1->getReturnType(), FD2->getReturnType()))
+      return false;
+
+    if (FD1->getNumParams() != FD2->getNumParams())
+      return false;
+
+    for (unsigned I = 0; I < FD1->getNumParams(); ++I)
+      if (!hasSameType(FD1->getParamDecl(I)->getOriginalType(),
+                       FD2->getParamDecl(I)->getOriginalType()))
+        return false;
+
+    return true;
+  }
+
   const CXXConstructorDecl *
   getCopyConstructorForExceptionObject(CXXRecordDecl *RD);
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 95ce4166ceb66..8a00fe21a08ce 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3811,8 +3811,6 @@ def warn_sme_locally_streaming_has_vl_args_returns : Warning<
   InGroup<AArch64SMEAttributes>, DefaultIgnore;
 def err_conflicting_attributes_arm_state : Error<
   "conflicting attributes for state '%0'">;
-def err_sme_streaming_cannot_be_multiversioned : Error<
-  "streaming function cannot be multi-versioned">;
 def err_unknown_arm_state : Error<
   "unknown state '%0'">;
 def err_missing_arm_state : Error<
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 7af9ea7105bb0..8f121ed0fe86c 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12451,8 +12451,7 @@ void ASTContext::forEachMultiversionedFunctionVersion(
   for (auto *CurDecl :
        FD->getDeclContext()->getRedeclContext()->lookup(FD->getDeclName())) {
     FunctionDecl *CurFD = CurDecl->getAsFunction()->getMostRecentDecl();
-    if (CurFD && hasSameType(CurFD->getType(), FD->getType()) &&
-        !SeenDecls.contains(CurFD)) {
+    if (CurFD && areFMVCompatible(CurFD, FD) && !SeenDecls.contains(CurFD)) {
       SeenDecls.insert(CurFD);
       Pred(CurFD);
     }
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index f60cc78be4f92..77331f0ca0997 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11014,6 +11014,9 @@ static bool AttrCompatibleWithMultiVersion(attr::Kind Kind,
   switch (Kind) {
   default:
     return false;
+  case attr::ArmLocallyStreaming:
+    return MVKind == MultiVersionKind::TargetVersion ||
+           MVKind == MultiVersionKind::TargetClones;
   case attr::Used:
     return MVKind == MultiVersionKind::Target;
   case attr::NonNull:
@@ -11150,7 +11153,24 @@ bool Sema::areMultiversionVariantFunctionsCompatible(
     FunctionType::ExtInfo OldTypeInfo = OldType->getExtInfo();
     FunctionType::ExtInfo NewTypeInfo = NewType->getExtInfo();
 
-    if (OldTypeInfo.getCC() != NewTypeInfo.getCC())
+    const auto *OldFPT = OldFD->getType()->getAs<FunctionProtoType>();
+    const auto *NewFPT = NewFD->getType()->getAs<FunctionProtoType>();
+
+    bool ArmStreamingCCMismatched = false;
+    // Locally streaming does not affect the calling convention.
+    if (OldFPT && NewFPT && !OldFD->hasAttr<ArmLocallyStreamingAttr>() &&
+        !NewFD->hasAttr<ArmLocallyStreamingAttr>()) {
+      unsigned Diff =
+          OldFPT->getAArch64SMEAttributes() ^ NewFPT->getAArch64SMEAttributes();
+      // Streaming versions cannot be mixed with non-streaming versions.
+      if (Diff & FunctionType::SME_PStateSMEnabledMask)
+        ArmStreamingCCMismatched = true;
+      // Streaming-compatible versions cannot be mixed with anything else.
+      if (Diff & FunctionType::SME_PStateSMCompatibleMask)
+        ArmStreamingCCMismatched = true;
+    }
+
+    if (OldTypeInfo.getCC() != NewTypeInfo.getCC() || ArmStreamingCCMismatched)
       return Diag(DiffDiagIDAt.first, DiffDiagIDAt.second) << CallingConv;
 
     QualType OldReturnType = OldType->getReturnType();
@@ -11170,9 +11190,8 @@ bool Sema::areMultiversionVariantFunctionsCompatible(
     if (!CLinkageMayDiffer && OldFD->isExternC() != NewFD->isExternC())
       return Diag(DiffDiagIDAt.first, DiffDiagIDAt.second) << LanguageLinkage;
 
-    if (CheckEquivalentExceptionSpec(
-            OldFD->getType()->getAs<FunctionProtoType>(), OldFD->getLocation(),
-            NewFD->getType()->getAs<FunctionProtoType>(), NewFD->getLocation()))
+    if (CheckEquivalentExceptionSpec(OldFPT, OldFD->getLocation(), NewFPT,
+                                     NewFD->getLocation()))
       return true;
   }
   return false;
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 10bacc17a07ca..e2eada24f9fcc 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3024,9 +3024,6 @@ bool Sema::checkTargetVersionAttr(SourceLocation LiteralLoc, Decl *D,
       return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
              << Unsupported << None << CurFeature << TargetVersion;
   }
-  if (IsArmStreamingFunction(cast<FunctionDecl>(D),
-                             /*IncludeLocallyStreaming=*/false))
-    return Diag(LiteralLoc, diag::err_sme_streaming_cannot_be_multiversioned);
   return false;
 }
 
@@ -3123,10 +3120,6 @@ bool Sema::checkTargetClonesAttrString(
           HasNotDefault = true;
         }
       }
-      if (IsArmStreamingFunction(cast<FunctionDecl>(D),
-                                 /*IncludeLocallyStreaming=*/false))
-        return Diag(LiteralLoc,
-                    diag::err_sme_streaming_cannot_be_multiversioned);
     } else {
       // Other targets ( currently X86 )
       if (Cur.starts_with("arch=")) {
diff --git a/clang/test/Sema/aarch64-fmv-streaming.c b/clang/test/Sema/aarch64-fmv-streaming.c
new file mode 100644
index 0000000000000..050ce5c8c803b
--- /dev/null
+++ b/clang/test/Sema/aarch64-fmv-streaming.c
@@ -0,0 +1,43 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme -Waarch64-sme-attributes -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme -Waarch64-sme-attributes -fsyntax-only -verify=expected-cpp -x c++ %s
+
+__attribute__((target_clones("sve", "simd"))) void ok_arm_streaming(void) __arm_streaming {}
+__arm_locally_streaming __attribute__((target_version("sme2"))) void ok_arm_streaming(void) {}
+__attribute__((target_version("default"))) void ok_arm_streaming(void) __arm_streaming {}
+
+__attribute__((target_clones("sve", "simd"))) void ok_arm_streaming_compatible(void) __arm_streaming_compatible {}
+__arm_locally_streaming __attribute__((target_version("sme2"))) void ok_arm_streaming_compatible(void) {}
+__attribute__((target_version("default"))) void ok_arm_streaming_compatible(void) __arm_streaming_compatible {}
+
+__arm_locally_streaming __attribute__((target_clones("sve", "simd"))) void ok_no_streaming(void) {}
+__attribute__((target_version("sme2"))) void ok_no_streaming(void) {}
+__attribute__((target_version("default"))) void ok_no_streaming(void) {}
+
+__attribute__((target_clones("sve", "simd"))) void bad_mixed_streaming(void) {}
+// expected-cpp-error at +2 {{multiversioned function declaration has a different calling convention}}
+// expected-error at +1 {{multiversioned function declaration has a different calling convention}}
+__attribute__((target_version("sme2"))) void bad_mixed_streaming(void) __arm_streaming {}
+// expected-cpp-error at +2 {{multiversioned function declaration has a different calling convention}}
+// expected-error at +1 {{multiversioned function declaration has a different calling convention}}
+__attribute__((target_version("default"))) void bad_mixed_streaming(void) __arm_streaming_compatible {}
+
+void n_caller(void) {
+  ok_arm_streaming();
+  ok_arm_streaming_compatible();
+  ok_no_streaming();
+  bad_mixed_streaming();
+}
+
+void s_caller(void) __arm_streaming {
+  ok_arm_streaming();
+  ok_arm_streaming_compatible();
+  ok_no_streaming();
+  bad_mixed_streaming();
+}
+
+void sc_caller(void) __arm_streaming_compatible {
+  ok_arm_streaming();
+  ok_arm_streaming_compatible();
+  ok_no_streaming();
+  bad_mixed_streaming();
+}
diff --git a/clang/test/Sema/aarch64-sme-func-attrs.c b/clang/test/Sema/aarch64-sme-func-attrs.c
index 6db39d6a71e36..0c263eb2610cf 100644
--- a/clang/test/Sema/aarch64-sme-func-attrs.c
+++ b/clang/test/Sema/aarch64-sme-func-attrs.c
@@ -455,48 +455,6 @@ void unimplemented_spill_fill_za(void (*share_zt0_only)(void) __arm_inout("zt0")
   share_zt0_only();
 }
 
-// expected-cpp-error at +2 {{streaming function cannot be multi-versioned}}
-// expected-error at +1 {{streaming function cannot be multi-versioned}}
-__attribute__((target_version("sme2")))
-void cannot_work_version(void) __arm_streaming {}
-// expected-cpp-error at +5 {{function declared 'void ()' was previously declared 'void () __arm_streaming', which has different SME function attributes}}
-// expected-cpp-note at -2 {{previous declaration is here}}
-// expected-error at +3 {{function declared 'void (void)' was previously declared 'void (void) __arm_streaming', which has different SME function attributes}}
-// expected-note at -4 {{previous declaration is here}}
-__attribute__((target_version("default")))
-void cannot_work_version(void) {}
-
-
-// expected-cpp-error at +2 {{streaming function cannot be multi-versioned}}
-// expected-error at +1 {{streaming function cannot be multi-versioned}}
-__attribute__((target_clones("sme2")))
-void cannot_work_clones(void) __arm_streaming {}
-
-
-__attribute__((target("sme2")))
-void just_fine_streaming(void) __arm_streaming {}
-__attribute__((target_version("sme2")))
-void just_fine(void) { just_fine_streaming(); }
-__attribute__((target_version("default")))
-void just_fine(void) {}
-
-
-__arm_locally_streaming
-__attribute__((target_version("sme2")))
-void incompatible_locally_streaming(void) {}
-// expected-error at -1 {{attribute 'target_version' multiversioning cannot be combined with attribute '__arm_locally_streaming'}}
-// expected-cpp-error at -2 {{attribute 'target_version' multiversioning cannot be combined with attribute '__arm_locally_streaming'}}
-__attribute__((target_version("default")))
-void incompatible_locally_streaming(void) {}
-
-
-void fmv_caller() {
-    cannot_work_version();
-    cannot_work_clones();
-    just_fine();
-    incompatible_locally_streaming();
-}
-
 void sme_streaming_with_vl_arg(__SVInt8_t a) __arm_streaming { }
 
 __SVInt8_t sme_streaming_returns_vl(void) __arm_streaming { __SVInt8_t r; return r; }

>From 00d97039a6dacd17beebce32b727e8c23900eeae Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Wed, 24 Jul 2024 12:28:46 +0100
Subject: [PATCH 2/5] Changes from last revision:

* Disregard declarations with different variadic type. Note that
  we are not diagnosing such differences, we just do not consider
  the two declarations part of the same declaration chain. As a
  result the diagnostic comes upon use: "ambiguous call". This
  is NFC.

* Added a sema test for variadic type mismatch.

* Added a codegen test for the calling conventions.
---
 clang/include/clang/AST/ASTContext.h       |   3 +
 clang/test/CodeGen/aarch64-fmv-streaming.c | 106 +++++++++++++++++++++
 clang/test/Sema/attr-target-version.c      |  12 +++
 3 files changed, 121 insertions(+)
 create mode 100644 clang/test/CodeGen/aarch64-fmv-streaming.c

diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index a86394a51db16..419104059838f 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -3194,6 +3194,9 @@ class ASTContext : public RefCountedBase<ASTContext> {
     if (!hasSameType(FD1->getReturnType(), FD2->getReturnType()))
       return false;
 
+    if (FD1->isVariadic() != FD2->isVariadic())
+      return false;
+
     if (FD1->getNumParams() != FD2->getNumParams())
       return false;
 
diff --git a/clang/test/CodeGen/aarch64-fmv-streaming.c b/clang/test/CodeGen/aarch64-fmv-streaming.c
new file mode 100644
index 0000000000000..e777a53b2f038
--- /dev/null
+++ b/clang/test/CodeGen/aarch64-fmv-streaming.c
@@ -0,0 +1,106 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme -emit-llvm -o - %s | FileCheck %s
+
+
+// CHECK-LABEL: define {{[^@]+}}@n_callee._Msve
+// CHECK-SAME: () #[[ATTR0:[0-9]+]] {
+//
+// CHECK-LABEL: define {{[^@]+}}@n_callee._Msimd
+// CHECK-SAME: () #[[ATTR1:[0-9]+]] {
+//
+__arm_locally_streaming __attribute__((target_clones("sve", "simd"))) void n_callee(void) {}
+// CHECK-LABEL: define {{[^@]+}}@n_callee._Msme2
+// CHECK-SAME: () #[[ATTR2:[0-9]+]] {
+//
+__attribute__((target_version("sme2"))) void n_callee(void) {}
+// CHECK-LABEL: define {{[^@]+}}@n_callee.default
+// CHECK-SAME: () #[[ATTR3:[0-9]+]] {
+//
+__attribute__((target_version("default"))) void n_callee(void) {}
+
+
+// CHECK-LABEL: define {{[^@]+}}@s_callee._Msve
+// CHECK-SAME: () #[[ATTR4:[0-9]+]] {
+//
+// CHECK-LABEL: define {{[^@]+}}@s_callee._Msimd
+// CHECK-SAME: () #[[ATTR5:[0-9]+]] {
+//
+__attribute__((target_clones("sve", "simd"))) void s_callee(void) __arm_streaming {}
+// CHECK-LABEL: define {{[^@]+}}@s_callee._Msme2
+// CHECK-SAME: () #[[ATTR6:[0-9]+]] {
+//
+__arm_locally_streaming __attribute__((target_version("sme2"))) void s_callee(void) {}
+// CHECK-LABEL: define {{[^@]+}}@s_callee.default
+// CHECK-SAME: () #[[ATTR7:[0-9]+]] {
+//
+__attribute__((target_version("default"))) void s_callee(void) __arm_streaming {}
+
+
+// CHECK-LABEL: define {{[^@]+}}@sc_callee._Msve
+// CHECK-SAME: () #[[ATTR8:[0-9]+]] {
+//
+// CHECK-LABEL: define {{[^@]+}}@sc_callee._Msimd
+// CHECK-SAME: () #[[ATTR9:[0-9]+]] {
+//
+__attribute__((target_clones("sve", "simd"))) void sc_callee(void) __arm_streaming_compatible {}
+// CHECK-LABEL: define {{[^@]+}}@sc_callee._Msme2
+// CHECK-SAME: () #[[ATTR6:[0-9]+]] {
+//
+__arm_locally_streaming __attribute__((target_version("sme2"))) void sc_callee(void) {}
+// CHECK-LABEL: define {{[^@]+}}@sc_callee.default
+// CHECK-SAME: () #[[ATTR10:[0-9]+]] {
+//
+__attribute__((target_version("default"))) void sc_callee(void) __arm_streaming_compatible {}
+
+
+// CHECK-LABEL: define {{[^@]+}}@n_caller
+// CHECK-SAME: () #[[ATTR3:[0-9]+]] {
+// CHECK:    call void @n_callee()
+// CHECK:    call void @s_callee() #[[ATTR11:[0-9]+]]
+// CHECK:    call void @sc_callee() #[[ATTR12:[0-9]+]]
+//
+void n_caller(void) {
+  n_callee();
+  s_callee();
+  sc_callee();
+}
+
+
+// CHECK-LABEL: define {{[^@]+}}@s_caller
+// CHECK-SAME: () #[[ATTR7:[0-9]+]] {
+// CHECK:    call void @n_callee()
+// CHECK:    call void @s_callee() #[[ATTR11]]
+// CHECK:    call void @sc_callee() #[[ATTR12]]
+//
+void s_caller(void) __arm_streaming {
+  n_callee();
+  s_callee();
+  sc_callee();
+}
+
+
+// CHECK-LABEL: define {{[^@]+}}@sc_caller
+// CHECK-SAME: () #[[ATTR10:[0-9]+]] {
+// CHECK:    call void @n_callee()
+// CHECK:    call void @s_callee() #[[ATTR11]]
+// CHECK:    call void @sc_callee() #[[ATTR12]]
+//
+void sc_caller(void) __arm_streaming_compatible {
+  n_callee();
+  s_callee();
+  sc_callee();
+}
+
+
+// CHECK: attributes #[[ATTR0:[0-9]+]] = {{.*}} "aarch64_pstate_sm_body"
+// CHECK: attributes #[[ATTR1:[0-9]+]] = {{.*}} "aarch64_pstate_sm_body"
+// CHECK: attributes #[[ATTR2:[0-9]+]] = {{.*}}
+// CHECK: attributes #[[ATTR3]] = {{.*}}
+// CHECK: attributes #[[ATTR4:[0-9]+]] = {{.*}} "aarch64_pstate_sm_enabled"
+// CHECK: attributes #[[ATTR5:[0-9]+]] = {{.*}} "aarch64_pstate_sm_enabled"
+// CHECK: attributes #[[ATTR6:[0-9]+]] = {{.*}} "aarch64_pstate_sm_body"
+// CHECK: attributes #[[ATTR7]] = {{.*}} "aarch64_pstate_sm_enabled"
+// CHECK: attributes #[[ATTR8:[0-9]+]] = {{.*}} "aarch64_pstate_sm_compatible"
+// CHECK: attributes #[[ATTR9:[0-9]+]] = {{.*}} "aarch64_pstate_sm_compatible"
+// CHECK: attributes #[[ATTR10]] = {{.*}} "aarch64_pstate_sm_compatible"
+// CHECK: attributes #[[ATTR11]] = {{.*}} "aarch64_pstate_sm_enabled"
+// CHECK: attributes #[[ATTR12]] = {{.*}} "aarch64_pstate_sm_compatible"
diff --git a/clang/test/Sema/attr-target-version.c b/clang/test/Sema/attr-target-version.c
index 88a927a58f991..91c89cfd1e7b0 100644
--- a/clang/test/Sema/attr-target-version.c
+++ b/clang/test/Sema/attr-target-version.c
@@ -112,3 +112,15 @@ int unspec_args_implicit_default_first();
 // expected-note at +1 {{function multiversioning caused by this declaration}}
 int __attribute__((target_version("aes"))) unspec_args_implicit_default_first() { return -1; }
 int __attribute__((target_version("default"))) unspec_args_implicit_default_first() { return 0; }
+
+void __attribute__((target_version("default"))) variadic_ok(int x, ...) {}
+void __attribute__((target_version("fp"))) variadic_ok(int x, ...) {}
+// expected-note at +1 {{candidate function}}
+void __attribute__((target_version("default"))) variadic_bad(int x) {}
+void __attribute__((target_version("fp"))) variadic_bad(int x, ...) {}
+
+void calls_variadic() {
+  variadic_ok(3);
+  //expected-error at +1 {{call to 'variadic_bad' is ambiguous}}
+  variadic_bad(3);
+}

>From 84de15796052e629a2276bcf1d502d1a8163e32b Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Wed, 24 Jul 2024 15:57:13 +0100
Subject: [PATCH 3/5] Changes from last revision:

Made __arm_locally_streaming require the same calling convention
as the rest of the callee versions and updated the tests.
---
 clang/lib/Sema/SemaDecl.cpp                |  4 +--
 clang/test/CodeGen/aarch64-fmv-streaming.c | 31 +++++++++++-----------
 clang/test/Sema/aarch64-fmv-streaming.c    |  7 +++--
 3 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 77331f0ca0997..4dc72063e54c0 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11157,9 +11157,7 @@ bool Sema::areMultiversionVariantFunctionsCompatible(
     const auto *NewFPT = NewFD->getType()->getAs<FunctionProtoType>();
 
     bool ArmStreamingCCMismatched = false;
-    // Locally streaming does not affect the calling convention.
-    if (OldFPT && NewFPT && !OldFD->hasAttr<ArmLocallyStreamingAttr>() &&
-        !NewFD->hasAttr<ArmLocallyStreamingAttr>()) {
+    if (OldFPT && NewFPT) {
       unsigned Diff =
           OldFPT->getAArch64SMEAttributes() ^ NewFPT->getAArch64SMEAttributes();
       // Streaming versions cannot be mixed with non-streaming versions.
diff --git a/clang/test/CodeGen/aarch64-fmv-streaming.c b/clang/test/CodeGen/aarch64-fmv-streaming.c
index e777a53b2f038..e549ccda59ad8 100644
--- a/clang/test/CodeGen/aarch64-fmv-streaming.c
+++ b/clang/test/CodeGen/aarch64-fmv-streaming.c
@@ -28,7 +28,7 @@ __attribute__((target_clones("sve", "simd"))) void s_callee(void) __arm_streamin
 // CHECK-LABEL: define {{[^@]+}}@s_callee._Msme2
 // CHECK-SAME: () #[[ATTR6:[0-9]+]] {
 //
-__arm_locally_streaming __attribute__((target_version("sme2"))) void s_callee(void) {}
+__arm_locally_streaming __attribute__((target_version("sme2"))) void s_callee(void) __arm_streaming {}
 // CHECK-LABEL: define {{[^@]+}}@s_callee.default
 // CHECK-SAME: () #[[ATTR7:[0-9]+]] {
 //
@@ -43,11 +43,11 @@ __attribute__((target_version("default"))) void s_callee(void) __arm_streaming {
 //
 __attribute__((target_clones("sve", "simd"))) void sc_callee(void) __arm_streaming_compatible {}
 // CHECK-LABEL: define {{[^@]+}}@sc_callee._Msme2
-// CHECK-SAME: () #[[ATTR6:[0-9]+]] {
+// CHECK-SAME: () #[[ATTR10:[0-9]+]] {
 //
-__arm_locally_streaming __attribute__((target_version("sme2"))) void sc_callee(void) {}
+__arm_locally_streaming __attribute__((target_version("sme2"))) void sc_callee(void) __arm_streaming_compatible {}
 // CHECK-LABEL: define {{[^@]+}}@sc_callee.default
-// CHECK-SAME: () #[[ATTR10:[0-9]+]] {
+// CHECK-SAME: () #[[ATTR11:[0-9]+]] {
 //
 __attribute__((target_version("default"))) void sc_callee(void) __arm_streaming_compatible {}
 
@@ -55,8 +55,8 @@ __attribute__((target_version("default"))) void sc_callee(void) __arm_streaming_
 // CHECK-LABEL: define {{[^@]+}}@n_caller
 // CHECK-SAME: () #[[ATTR3:[0-9]+]] {
 // CHECK:    call void @n_callee()
-// CHECK:    call void @s_callee() #[[ATTR11:[0-9]+]]
-// CHECK:    call void @sc_callee() #[[ATTR12:[0-9]+]]
+// CHECK:    call void @s_callee() #[[ATTR12:[0-9]+]]
+// CHECK:    call void @sc_callee() #[[ATTR13:[0-9]+]]
 //
 void n_caller(void) {
   n_callee();
@@ -68,8 +68,8 @@ void n_caller(void) {
 // CHECK-LABEL: define {{[^@]+}}@s_caller
 // CHECK-SAME: () #[[ATTR7:[0-9]+]] {
 // CHECK:    call void @n_callee()
-// CHECK:    call void @s_callee() #[[ATTR11]]
-// CHECK:    call void @sc_callee() #[[ATTR12]]
+// CHECK:    call void @s_callee() #[[ATTR12]]
+// CHECK:    call void @sc_callee() #[[ATTR13]]
 //
 void s_caller(void) __arm_streaming {
   n_callee();
@@ -79,10 +79,10 @@ void s_caller(void) __arm_streaming {
 
 
 // CHECK-LABEL: define {{[^@]+}}@sc_caller
-// CHECK-SAME: () #[[ATTR10:[0-9]+]] {
+// CHECK-SAME: () #[[ATTR11:[0-9]+]] {
 // CHECK:    call void @n_callee()
-// CHECK:    call void @s_callee() #[[ATTR11]]
-// CHECK:    call void @sc_callee() #[[ATTR12]]
+// CHECK:    call void @s_callee() #[[ATTR12]]
+// CHECK:    call void @sc_callee() #[[ATTR13]]
 //
 void sc_caller(void) __arm_streaming_compatible {
   n_callee();
@@ -97,10 +97,11 @@ void sc_caller(void) __arm_streaming_compatible {
 // CHECK: attributes #[[ATTR3]] = {{.*}}
 // CHECK: attributes #[[ATTR4:[0-9]+]] = {{.*}} "aarch64_pstate_sm_enabled"
 // CHECK: attributes #[[ATTR5:[0-9]+]] = {{.*}} "aarch64_pstate_sm_enabled"
-// CHECK: attributes #[[ATTR6:[0-9]+]] = {{.*}} "aarch64_pstate_sm_body"
+// CHECK: attributes #[[ATTR6:[0-9]+]] = {{.*}} "aarch64_pstate_sm_body" "aarch64_pstate_sm_enabled"
 // CHECK: attributes #[[ATTR7]] = {{.*}} "aarch64_pstate_sm_enabled"
 // CHECK: attributes #[[ATTR8:[0-9]+]] = {{.*}} "aarch64_pstate_sm_compatible"
 // CHECK: attributes #[[ATTR9:[0-9]+]] = {{.*}} "aarch64_pstate_sm_compatible"
-// CHECK: attributes #[[ATTR10]] = {{.*}} "aarch64_pstate_sm_compatible"
-// CHECK: attributes #[[ATTR11]] = {{.*}} "aarch64_pstate_sm_enabled"
-// CHECK: attributes #[[ATTR12]] = {{.*}} "aarch64_pstate_sm_compatible"
+// CHECK: attributes #[[ATTR10]] = {{.*}} "aarch64_pstate_sm_body" "aarch64_pstate_sm_compatible"
+// CHECK: attributes #[[ATTR11]] = {{.*}} "aarch64_pstate_sm_compatible"
+// CHECK: attributes #[[ATTR12]] = {{.*}} "aarch64_pstate_sm_enabled"
+// CHECK: attributes #[[ATTR13]] = {{.*}} "aarch64_pstate_sm_compatible"
diff --git a/clang/test/Sema/aarch64-fmv-streaming.c b/clang/test/Sema/aarch64-fmv-streaming.c
index 050ce5c8c803b..93b7656216c0c 100644
--- a/clang/test/Sema/aarch64-fmv-streaming.c
+++ b/clang/test/Sema/aarch64-fmv-streaming.c
@@ -2,11 +2,11 @@
 // RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sme -Waarch64-sme-attributes -fsyntax-only -verify=expected-cpp -x c++ %s
 
 __attribute__((target_clones("sve", "simd"))) void ok_arm_streaming(void) __arm_streaming {}
-__arm_locally_streaming __attribute__((target_version("sme2"))) void ok_arm_streaming(void) {}
+__arm_locally_streaming __attribute__((target_version("sme2"))) void ok_arm_streaming(void) __arm_streaming {}
 __attribute__((target_version("default"))) void ok_arm_streaming(void) __arm_streaming {}
 
 __attribute__((target_clones("sve", "simd"))) void ok_arm_streaming_compatible(void) __arm_streaming_compatible {}
-__arm_locally_streaming __attribute__((target_version("sme2"))) void ok_arm_streaming_compatible(void) {}
+__arm_locally_streaming __attribute__((target_version("sme2"))) void ok_arm_streaming_compatible(void) __arm_streaming_compatible {}
 __attribute__((target_version("default"))) void ok_arm_streaming_compatible(void) __arm_streaming_compatible {}
 
 __arm_locally_streaming __attribute__((target_clones("sve", "simd"))) void ok_no_streaming(void) {}
@@ -20,6 +20,9 @@ __attribute__((target_version("sme2"))) void bad_mixed_streaming(void) __arm_str
 // expected-cpp-error at +2 {{multiversioned function declaration has a different calling convention}}
 // expected-error at +1 {{multiversioned function declaration has a different calling convention}}
 __attribute__((target_version("default"))) void bad_mixed_streaming(void) __arm_streaming_compatible {}
+// expected-cpp-error at +2 {{multiversioned function declaration has a different calling convention}}
+// expected-error at +1 {{multiversioned function declaration has a different calling convention}}
+__arm_locally_streaming __attribute__((target_version("dotprod"))) void bad_mixed_streaming(void) __arm_streaming {}
 
 void n_caller(void) {
   ok_arm_streaming();

>From 196fb42d2ef10cc6b3c9732c2612d2cd2973d340 Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Wed, 24 Jul 2024 20:22:02 +0100
Subject: [PATCH 4/5] Changes fro last revision:

Combined two separate SME_PState bitmask checks into one as suggested.
---
 clang/lib/Sema/SemaDecl.cpp | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 4dc72063e54c0..01231f8e385ef 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11160,11 +11160,10 @@ bool Sema::areMultiversionVariantFunctionsCompatible(
     if (OldFPT && NewFPT) {
       unsigned Diff =
           OldFPT->getAArch64SMEAttributes() ^ NewFPT->getAArch64SMEAttributes();
-      // Streaming versions cannot be mixed with non-streaming versions.
-      if (Diff & FunctionType::SME_PStateSMEnabledMask)
-        ArmStreamingCCMismatched = true;
-      // Streaming-compatible versions cannot be mixed with anything else.
-      if (Diff & FunctionType::SME_PStateSMCompatibleMask)
+      // Arm-streaming, arm-streaming-compatible and non-streaming versions
+      // cannot be mixed.
+      if (Diff & (FunctionType::SME_PStateSMEnabledMask |
+                  FunctionType::SME_PStateSMCompatibleMask))
         ArmStreamingCCMismatched = true;
     }
 

>From bb412b723139a7a7f20f768857d9d4c656ac6fbb Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Thu, 25 Jul 2024 17:54:14 +0100
Subject: [PATCH 5/5] Changes from last revision:

* Replaced areFMVCompatible with hasSameType. Looks like this change was
  unnecessary in the first place. Most likely a residue from my WIP
  before I raised the PR. Thanks Sander for finding this!

* Removed the corresponding sema test for variadic type mismatch.
---
 clang/include/clang/AST/ASTContext.h  | 19 -------------------
 clang/lib/AST/ASTContext.cpp          |  3 ++-
 clang/test/Sema/attr-target-version.c | 12 ------------
 3 files changed, 2 insertions(+), 32 deletions(-)

diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 419104059838f..6d1c8ca8a2f96 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -3189,25 +3189,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
       const FunctionDecl *FD,
       llvm::function_ref<void(FunctionDecl *)> Pred) const;
 
-  bool areFMVCompatible(const FunctionDecl *FD1,
-                        const FunctionDecl *FD2) const {
-    if (!hasSameType(FD1->getReturnType(), FD2->getReturnType()))
-      return false;
-
-    if (FD1->isVariadic() != FD2->isVariadic())
-      return false;
-
-    if (FD1->getNumParams() != FD2->getNumParams())
-      return false;
-
-    for (unsigned I = 0; I < FD1->getNumParams(); ++I)
-      if (!hasSameType(FD1->getParamDecl(I)->getOriginalType(),
-                       FD2->getParamDecl(I)->getOriginalType()))
-        return false;
-
-    return true;
-  }
-
   const CXXConstructorDecl *
   getCopyConstructorForExceptionObject(CXXRecordDecl *RD);
 
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 8f121ed0fe86c..7af9ea7105bb0 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -12451,7 +12451,8 @@ void ASTContext::forEachMultiversionedFunctionVersion(
   for (auto *CurDecl :
        FD->getDeclContext()->getRedeclContext()->lookup(FD->getDeclName())) {
     FunctionDecl *CurFD = CurDecl->getAsFunction()->getMostRecentDecl();
-    if (CurFD && areFMVCompatible(CurFD, FD) && !SeenDecls.contains(CurFD)) {
+    if (CurFD && hasSameType(CurFD->getType(), FD->getType()) &&
+        !SeenDecls.contains(CurFD)) {
       SeenDecls.insert(CurFD);
       Pred(CurFD);
     }
diff --git a/clang/test/Sema/attr-target-version.c b/clang/test/Sema/attr-target-version.c
index 91c89cfd1e7b0..88a927a58f991 100644
--- a/clang/test/Sema/attr-target-version.c
+++ b/clang/test/Sema/attr-target-version.c
@@ -112,15 +112,3 @@ int unspec_args_implicit_default_first();
 // expected-note at +1 {{function multiversioning caused by this declaration}}
 int __attribute__((target_version("aes"))) unspec_args_implicit_default_first() { return -1; }
 int __attribute__((target_version("default"))) unspec_args_implicit_default_first() { return 0; }
-
-void __attribute__((target_version("default"))) variadic_ok(int x, ...) {}
-void __attribute__((target_version("fp"))) variadic_ok(int x, ...) {}
-// expected-note at +1 {{candidate function}}
-void __attribute__((target_version("default"))) variadic_bad(int x) {}
-void __attribute__((target_version("fp"))) variadic_bad(int x, ...) {}
-
-void calls_variadic() {
-  variadic_ok(3);
-  //expected-error at +1 {{call to 'variadic_bad' is ambiguous}}
-  variadic_bad(3);
-}



More information about the llvm-branch-commits mailing list