[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)

Alexandros Lamprineas via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 1 02:00:39 PDT 2024


https://github.com/labrinea updated https://github.com/llvm/llvm-project/pull/96628

>From ff4635208e9cd83c6735c95ebf12125ca737029a Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Tue, 25 Jun 2024 00:27:45 +0100
Subject: [PATCH 1/3] [clang][FMV] Do not omit explicit default target_version
 attribute.

Fixes a crash and cleans up some dead code.

namespace Foo {
int bar();
__attribute((target_version("default"))) int bar() { return 0; }
__attribute((target_version("mops"))) int bar() { return 1; }
}

$ clang++ --target=aarch64-linux-gnu --rtlib=compiler-rt fmv.cpp

None multiversion type isn't valid here
UNREACHABLE executed at clang/lib/CodeGen/CodeGenModule.cpp:1840!
...
getMangledNameImpl
clang::CodeGen::CodeGenModule::getMangledName
clang::CodeGen::CodeGenModule::EmitGlobal
---
 clang/include/clang/Sema/Sema.h          |  3 +-
 clang/lib/Sema/SemaDecl.cpp              | 45 +++++-------
 clang/lib/Sema/SemaDeclAttr.cpp          | 16 ++---
 clang/test/CodeGen/attr-target-version.c | 88 ++++++++++++------------
 clang/test/CodeGenCXX/fmv-namespace.cpp  | 60 ++++++++++++----
 clang/test/Sema/attr-target-version.c    |  5 +-
 6 files changed, 118 insertions(+), 99 deletions(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 2e7af0f691cbb..31bb81705a742 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3786,8 +3786,7 @@ class Sema final : public SemaBase {
                                 StringRef Name);
 
   bool checkTargetAttr(SourceLocation LiteralLoc, StringRef Str);
-  bool checkTargetVersionAttr(SourceLocation LiteralLoc, Decl *D,
-                              StringRef &Str, bool &isDefault);
+  bool checkTargetVersionAttr(SourceLocation Loc, Decl *D, StringRef Str);
   bool checkTargetClonesAttrString(
       SourceLocation LiteralLoc, StringRef Str, const StringLiteral *Literal,
       Decl *D, bool &HasDefault, bool &HasCommas, bool &HasNotDefault,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 029ccf944c513..572c46eed1aaa 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11465,6 +11465,10 @@ static bool CheckMultiVersionFirstFunction(Sema &S, FunctionDecl *FD) {
   // otherwise it is treated as a normal function.
   if (TA && !TA->isDefaultVersion())
     return false;
+  // The target_version attribute only causes Multiversioning if this
+  // declaration is NOT the default version.
+  if (TVA && TVA->isDefaultVersion())
+    return false;
 
   if ((TA || TVA) && CheckMultiVersionValue(S, FD)) {
     FD->setInvalidDecl();
@@ -11498,11 +11502,9 @@ static void patchDefaultTargetVersion(FunctionDecl *From, FunctionDecl *To) {
 
   if (MVKindTo == MultiVersionKind::None &&
       (MVKindFrom == MultiVersionKind::TargetVersion ||
-       MVKindFrom == MultiVersionKind::TargetClones)) {
-    To->setIsMultiVersion();
+       MVKindFrom == MultiVersionKind::TargetClones))
     To->addAttr(TargetVersionAttr::CreateImplicit(
         To->getASTContext(), "default", To->getSourceRange()));
-  }
 }
 
 static bool CheckTargetCausesMultiVersioning(Sema &S, FunctionDecl *OldFD,
@@ -11523,10 +11525,16 @@ static bool CheckTargetCausesMultiVersioning(Sema &S, FunctionDecl *OldFD,
   const auto *OldTVA = OldFD->getAttr<TargetVersionAttr>();
   // If the old decl is NOT MultiVersioned yet, and we don't cause that
   // to change, this is a simple redeclaration.
-  if ((NewTA && !NewTA->isDefaultVersion() &&
-       (!OldTA || OldTA->getFeaturesStr() == NewTA->getFeaturesStr())) ||
-      (NewTVA && !NewTVA->isDefaultVersion() &&
-       (!OldTVA || OldTVA->getName() == NewTVA->getName())))
+  if (NewTA && !NewTA->isDefaultVersion() &&
+      (!OldTA || OldTA->getFeaturesStr() == NewTA->getFeaturesStr()))
+    return false;
+
+  // The target_version attribute only causes Multiversioning if this
+  // declaration is NOT the default version. Moreover, the old declaration
+  // must be the default version (either explicitly via the attribute,
+  // or implicitly without it).
+  if (NewTVA &&
+      (NewTVA->isDefaultVersion() || (OldTVA && !OldTVA->isDefaultVersion())))
     return false;
 
   // Otherwise, this decl causes MultiVersioning.
@@ -11543,8 +11551,7 @@ static bool CheckTargetCausesMultiVersioning(Sema &S, FunctionDecl *OldFD,
   }
 
   // If this is 'default', permit the forward declaration.
-  if ((NewTA && NewTA->isDefaultVersion() && !OldTA) ||
-      (NewTVA && NewTVA->isDefaultVersion() && !OldTVA)) {
+  if (NewTA && NewTA->isDefaultVersion() && !OldTA) {
     Redeclaration = true;
     OldDecl = OldFD;
     OldFD->setIsMultiVersion();
@@ -11947,24 +11954,8 @@ static bool CheckMultiVersionFunction(Sema &S, FunctionDecl *NewFD,
 
   FunctionDecl *OldFD = OldDecl->getAsFunction();
 
-  if (!OldFD->isMultiVersion() && MVKind == MultiVersionKind::None) {
-    if (NewTVA || !OldFD->getAttr<TargetVersionAttr>())
-      return false;
-    if (!NewFD->getType()->getAs<FunctionProtoType>()) {
-      // Multiversion declaration doesn't have prototype.
-      S.Diag(NewFD->getLocation(), diag::err_multiversion_noproto);
-      NewFD->setInvalidDecl();
-    } else {
-      // No "target_version" attribute is equivalent to "default" attribute.
-      NewFD->addAttr(TargetVersionAttr::CreateImplicit(
-          S.Context, "default", NewFD->getSourceRange()));
-      NewFD->setIsMultiVersion();
-      OldFD->setIsMultiVersion();
-      OldDecl = OldFD;
-      Redeclaration = true;
-    }
-    return true;
-  }
+  if (!OldFD->isMultiVersion() && MVKind == MultiVersionKind::None)
+    return false;
 
   // Multiversioned redeclarations aren't allowed to omit the attribute, except
   // for target_clones and target_version.
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index ce6b5b1ff6f93..82e447a79a6b2 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3029,12 +3029,10 @@ bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) {
 
 // Check Target Version attrs
 bool Sema::checkTargetVersionAttr(SourceLocation LiteralLoc, Decl *D,
-                                  StringRef &AttrStr, bool &isDefault) {
+                                  StringRef AttrStr) {
   enum FirstParam { Unsupported };
   enum SecondParam { None };
   enum ThirdParam { Target, TargetClones, TargetVersion };
-  if (AttrStr.trim() == "default")
-    isDefault = true;
   llvm::SmallVector<StringRef, 8> Features;
   AttrStr.split(Features, "+");
   for (auto &CurFeature : Features) {
@@ -3054,16 +3052,12 @@ bool Sema::checkTargetVersionAttr(SourceLocation LiteralLoc, Decl *D,
 static void handleTargetVersionAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
   StringRef Str;
   SourceLocation LiteralLoc;
-  bool isDefault = false;
   if (!S.checkStringLiteralArgumentAttr(AL, 0, Str, &LiteralLoc) ||
-      S.checkTargetVersionAttr(LiteralLoc, D, Str, isDefault))
+      S.checkTargetVersionAttr(LiteralLoc, D, Str))
     return;
-  // Do not create default only target_version attribute
-  if (!isDefault) {
-    TargetVersionAttr *NewAttr =
-        ::new (S.Context) TargetVersionAttr(S.Context, AL, Str);
-    D->addAttr(NewAttr);
-  }
+  TargetVersionAttr *NewAttr =
+      ::new (S.Context) TargetVersionAttr(S.Context, AL, Str);
+  D->addAttr(NewAttr);
 }
 
 static void handleTargetAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
diff --git a/clang/test/CodeGen/attr-target-version.c b/clang/test/CodeGen/attr-target-version.c
index 024aafffca629..4edfc5408fae7 100644
--- a/clang/test/CodeGen/attr-target-version.c
+++ b/clang/test/CodeGen/attr-target-version.c
@@ -428,13 +428,6 @@ int caller(void) { return used_def_without_default_decl() + used_decl_without_de
 //
 //
 // CHECK: Function Attrs: noinline nounwind optnone
-// CHECK-LABEL: define {{[^@]+}}@fmv_default
-// CHECK-SAME: () #[[ATTR11]] {
-// CHECK-NEXT:  entry:
-// CHECK-NEXT:    ret i32 111
-//
-//
-// CHECK: Function Attrs: noinline nounwind optnone
 // CHECK-LABEL: define {{[^@]+}}@fmv_c._Mssbs
 // CHECK-SAME: () #[[ATTR11]] {
 // CHECK-NEXT:  entry:
@@ -637,22 +630,18 @@ int caller(void) { return used_def_without_default_decl() + used_decl_without_de
 //
 //
 // CHECK: Function Attrs: noinline nounwind optnone
-// CHECK-LABEL: define {{[^@]+}}@recur
+// CHECK-LABEL: define {{[^@]+}}@fmv_default
 // CHECK-SAME: () #[[ATTR11]] {
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:    call void @reca()
-// CHECK-NEXT:    ret void
+// CHECK-NEXT:    ret i32 111
 //
 //
 // CHECK: Function Attrs: noinline nounwind optnone
-// CHECK-LABEL: define {{[^@]+}}@main
+// CHECK-LABEL: define {{[^@]+}}@recur
 // CHECK-SAME: () #[[ATTR11]] {
 // CHECK-NEXT:  entry:
-// CHECK-NEXT:    [[RETVAL:%.*]] = alloca i32, align 4
-// CHECK-NEXT:    store i32 0, ptr [[RETVAL]], align 4
-// CHECK-NEXT:    call void @recur()
-// CHECK-NEXT:    [[CALL:%.*]] = call i32 @goo()
-// CHECK-NEXT:    ret i32 [[CALL]]
+// CHECK-NEXT:    call void @reca()
+// CHECK-NEXT:    ret void
 //
 //
 // CHECK: Function Attrs: noinline nounwind optnone
@@ -818,6 +807,17 @@ int caller(void) { return used_def_without_default_decl() + used_decl_without_de
 //
 //
 // CHECK: Function Attrs: noinline nounwind optnone
+// CHECK-LABEL: define {{[^@]+}}@main
+// CHECK-SAME: () #[[ATTR11]] {
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[RETVAL:%.*]] = alloca i32, align 4
+// CHECK-NEXT:    store i32 0, ptr [[RETVAL]], align 4
+// CHECK-NEXT:    call void @recur()
+// CHECK-NEXT:    [[CALL:%.*]] = call i32 @goo()
+// CHECK-NEXT:    ret i32 [[CALL]]
+//
+//
+// CHECK: Function Attrs: noinline nounwind optnone
 // CHECK-LABEL: define {{[^@]+}}@fmv_inline._Mf64mmMpmullMsha1
 // CHECK-SAME: () #[[ATTR22:[0-9]+]] {
 // CHECK-NEXT:  entry:
@@ -1020,20 +1020,6 @@ int caller(void) { return used_def_without_default_decl() + used_decl_without_de
 //
 //
 // CHECK-NOFMV: Function Attrs: noinline nounwind optnone
-// CHECK-NOFMV-LABEL: define {{[^@]+}}@fmv_default
-// CHECK-NOFMV-SAME: () #[[ATTR0]] {
-// CHECK-NOFMV-NEXT:  entry:
-// CHECK-NOFMV-NEXT:    ret i32 111
-//
-//
-// CHECK-NOFMV: Function Attrs: noinline nounwind optnone
-// CHECK-NOFMV-LABEL: define {{[^@]+}}@fmv_c
-// CHECK-NOFMV-SAME: () #[[ATTR0]] {
-// CHECK-NOFMV-NEXT:  entry:
-// CHECK-NOFMV-NEXT:    ret void
-//
-//
-// CHECK-NOFMV: Function Attrs: noinline nounwind optnone
 // CHECK-NOFMV-LABEL: define {{[^@]+}}@goo
 // CHECK-NOFMV-SAME: () #[[ATTR0]] {
 // CHECK-NOFMV-NEXT:  entry:
@@ -1053,22 +1039,25 @@ int caller(void) { return used_def_without_default_decl() + used_decl_without_de
 //
 //
 // CHECK-NOFMV: Function Attrs: noinline nounwind optnone
-// CHECK-NOFMV-LABEL: define {{[^@]+}}@recur
+// CHECK-NOFMV-LABEL: define {{[^@]+}}@fmv_c
 // CHECK-NOFMV-SAME: () #[[ATTR0]] {
 // CHECK-NOFMV-NEXT:  entry:
-// CHECK-NOFMV-NEXT:    call void @reca()
 // CHECK-NOFMV-NEXT:    ret void
 //
 //
 // CHECK-NOFMV: Function Attrs: noinline nounwind optnone
-// CHECK-NOFMV-LABEL: define {{[^@]+}}@main
+// CHECK-NOFMV-LABEL: define {{[^@]+}}@fmv_default
 // CHECK-NOFMV-SAME: () #[[ATTR0]] {
 // CHECK-NOFMV-NEXT:  entry:
-// CHECK-NOFMV-NEXT:    [[RETVAL:%.*]] = alloca i32, align 4
-// CHECK-NOFMV-NEXT:    store i32 0, ptr [[RETVAL]], align 4
-// CHECK-NOFMV-NEXT:    call void @recur()
-// CHECK-NOFMV-NEXT:    [[CALL:%.*]] = call i32 @goo()
-// CHECK-NOFMV-NEXT:    ret i32 [[CALL]]
+// CHECK-NOFMV-NEXT:    ret i32 111
+//
+//
+// CHECK-NOFMV: Function Attrs: noinline nounwind optnone
+// CHECK-NOFMV-LABEL: define {{[^@]+}}@recur
+// CHECK-NOFMV-SAME: () #[[ATTR0]] {
+// CHECK-NOFMV-NEXT:  entry:
+// CHECK-NOFMV-NEXT:    call void @reca()
+// CHECK-NOFMV-NEXT:    ret void
 //
 //
 // CHECK-NOFMV: Function Attrs: noinline nounwind optnone
@@ -1089,31 +1078,42 @@ int caller(void) { return used_def_without_default_decl() + used_decl_without_de
 //
 //
 // CHECK-NOFMV: Function Attrs: noinline nounwind optnone
-// CHECK-NOFMV-LABEL: define {{[^@]+}}@unused_with_default_def
+// CHECK-NOFMV-LABEL: define {{[^@]+}}@unused_with_implicit_default_def
 // CHECK-NOFMV-SAME: () #[[ATTR0]] {
 // CHECK-NOFMV-NEXT:  entry:
 // CHECK-NOFMV-NEXT:    ret i32 1
 //
 //
 // CHECK-NOFMV: Function Attrs: noinline nounwind optnone
-// CHECK-NOFMV-LABEL: define {{[^@]+}}@unused_with_implicit_default_def
+// CHECK-NOFMV-LABEL: define {{[^@]+}}@unused_with_implicit_forward_default_def
 // CHECK-NOFMV-SAME: () #[[ATTR0]] {
 // CHECK-NOFMV-NEXT:  entry:
-// CHECK-NOFMV-NEXT:    ret i32 1
+// CHECK-NOFMV-NEXT:    ret i32 0
 //
 //
 // CHECK-NOFMV: Function Attrs: noinline nounwind optnone
-// CHECK-NOFMV-LABEL: define {{[^@]+}}@unused_with_implicit_forward_default_def
+// CHECK-NOFMV-LABEL: define {{[^@]+}}@default_def_with_version_decls
 // CHECK-NOFMV-SAME: () #[[ATTR0]] {
 // CHECK-NOFMV-NEXT:  entry:
 // CHECK-NOFMV-NEXT:    ret i32 0
 //
 //
 // CHECK-NOFMV: Function Attrs: noinline nounwind optnone
-// CHECK-NOFMV-LABEL: define {{[^@]+}}@default_def_with_version_decls
+// CHECK-NOFMV-LABEL: define {{[^@]+}}@main
 // CHECK-NOFMV-SAME: () #[[ATTR0]] {
 // CHECK-NOFMV-NEXT:  entry:
-// CHECK-NOFMV-NEXT:    ret i32 0
+// CHECK-NOFMV-NEXT:    [[RETVAL:%.*]] = alloca i32, align 4
+// CHECK-NOFMV-NEXT:    store i32 0, ptr [[RETVAL]], align 4
+// CHECK-NOFMV-NEXT:    call void @recur()
+// CHECK-NOFMV-NEXT:    [[CALL:%.*]] = call i32 @goo()
+// CHECK-NOFMV-NEXT:    ret i32 [[CALL]]
+//
+//
+// CHECK-NOFMV: Function Attrs: noinline nounwind optnone
+// CHECK-NOFMV-LABEL: define {{[^@]+}}@unused_with_default_def
+// CHECK-NOFMV-SAME: () #[[ATTR0]] {
+// CHECK-NOFMV-NEXT:  entry:
+// CHECK-NOFMV-NEXT:    ret i32 1
 //
 //.
 // CHECK: attributes #[[ATTR0]] = { noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+flagm,+fp-armv8,+fp16fml,+fullfp16,+neon,+rand,-v9.5a" }
diff --git a/clang/test/CodeGenCXX/fmv-namespace.cpp b/clang/test/CodeGenCXX/fmv-namespace.cpp
index 193f01db4c5d3..abfff1a74f86a 100644
--- a/clang/test/CodeGenCXX/fmv-namespace.cpp
+++ b/clang/test/CodeGenCXX/fmv-namespace.cpp
@@ -17,25 +17,26 @@ int __attribute((target_version("sve"))) foo() { return 2; }
 
 int baz() { return OtherName::foo(); }
 
+namespace Foo {
+int bar();
+__attribute((target_version("default"))) int bar() { return 0; }
+__attribute((target_version("mops"))) int bar() { return 1; }
+}
+
 //.
 // CHECK: @__aarch64_cpu_features = external dso_local global { i64 }
 // CHECK: @_ZN4Name3fooEv = weak_odr ifunc i32 (), ptr @_ZN4Name3fooEv.resolver
 // CHECK: @_ZN9OtherName3fooEv = weak_odr ifunc i32 (), ptr @_ZN9OtherName3fooEv.resolver
+// CHECK: @_ZN3Foo3barEv = weak_odr ifunc i32 (), ptr @_ZN3Foo3barEv.resolver
 //.
-// CHECK-LABEL: define dso_local noundef i32 @_ZN4Name3fooEv.default(
-// CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
-// CHECK-NEXT:  [[ENTRY:.*:]]
-// CHECK-NEXT:    ret i32 0
-//
-//
 // CHECK-LABEL: define dso_local noundef i32 @_ZN4Name3fooEv._Msve(
-// CHECK-SAME: ) #[[ATTR1:[0-9]+]] {
+// CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
 // CHECK-NEXT:  [[ENTRY:.*:]]
 // CHECK-NEXT:    ret i32 1
 //
 //
 // CHECK-LABEL: define dso_local noundef i32 @_Z3barv(
-// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-SAME: ) #[[ATTR1:[0-9]+]] {
 // CHECK-NEXT:  [[ENTRY:.*:]]
 // CHECK-NEXT:    [[CALL:%.*]] = call noundef i32 @_ZN4Name3fooEv()
 // CHECK-NEXT:    ret i32 [[CALL]]
@@ -56,13 +57,13 @@ int baz() { return OtherName::foo(); }
 //
 //
 // CHECK-LABEL: define dso_local noundef i32 @_ZN9OtherName3fooEv._Msve(
-// CHECK-SAME: ) #[[ATTR1]] {
+// CHECK-SAME: ) #[[ATTR0]] {
 // CHECK-NEXT:  [[ENTRY:.*:]]
 // CHECK-NEXT:    ret i32 2
 //
 //
 // CHECK-LABEL: define dso_local noundef i32 @_Z3bazv(
-// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-SAME: ) #[[ATTR1]] {
 // CHECK-NEXT:  [[ENTRY:.*:]]
 // CHECK-NEXT:    [[CALL:%.*]] = call noundef i32 @_ZN9OtherName3fooEv()
 // CHECK-NEXT:    ret i32 [[CALL]]
@@ -81,10 +82,43 @@ int baz() { return OtherName::foo(); }
 // CHECK:       [[RESOLVER_ELSE]]:
 // CHECK-NEXT:    ret ptr @_ZN9OtherName3fooEv.default
 //
+//
+// CHECK-LABEL: define dso_local noundef i32 @_ZN3Foo3barEv.default(
+// CHECK-SAME: ) #[[ATTR1]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    ret i32 0
+//
+//
+// CHECK-LABEL: define dso_local noundef i32 @_ZN3Foo3barEv._Mmops(
+// CHECK-SAME: ) #[[ATTR2:[0-9]+]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    ret i32 1
+//
+//
+// CHECK-LABEL: define dso_local noundef i32 @_ZN4Name3fooEv.default(
+// CHECK-SAME: ) #[[ATTR1]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    ret i32 0
+//
+//
+// CHECK-LABEL: define weak_odr ptr @_ZN3Foo3barEv.resolver() comdat {
+// CHECK-NEXT:  [[RESOLVER_ENTRY:.*:]]
+// CHECK-NEXT:    call void @__init_cpu_features_resolver()
+// CHECK-NEXT:    [[TMP0:%.*]] = load i64, ptr @__aarch64_cpu_features, align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = and i64 [[TMP0]], 576460752303423488
+// CHECK-NEXT:    [[TMP2:%.*]] = icmp eq i64 [[TMP1]], 576460752303423488
+// CHECK-NEXT:    [[TMP3:%.*]] = and i1 true, [[TMP2]]
+// CHECK-NEXT:    br i1 [[TMP3]], label %[[RESOLVER_RETURN:.*]], label %[[RESOLVER_ELSE:.*]]
+// CHECK:       [[RESOLVER_RETURN]]:
+// CHECK-NEXT:    ret ptr @_ZN3Foo3barEv._Mmops
+// CHECK:       [[RESOLVER_ELSE]]:
+// CHECK-NEXT:    ret ptr @_ZN3Foo3barEv.default
+//
 //.
-// CHECK: attributes #[[ATTR0]] = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
-// CHECK: attributes #[[ATTR1]] = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+fullfp16,+neon,+sve" }
-// CHECK: attributes #[[ATTR2:[0-9]+]] = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
+// CHECK: attributes #[[ATTR0]] = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+fp-armv8,+fullfp16,+neon,+sve" }
+// CHECK: attributes #[[ATTR1]] = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
+// CHECK: attributes #[[ATTR2]] = { mustprogress noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+mops" }
+// CHECK: attributes #[[ATTR3:[0-9]+]] = { "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
 //.
 // CHECK: [[META0:![0-9]+]] = !{i32 1, !"wchar_size", i32 4}
 // CHECK: [[META1:![0-9]+]] = !{!"{{.*}}clang version {{.*}}"}
diff --git a/clang/test/Sema/attr-target-version.c b/clang/test/Sema/attr-target-version.c
index cd5be459456eb..8de3eeeaff8f5 100644
--- a/clang/test/Sema/attr-target-version.c
+++ b/clang/test/Sema/attr-target-version.c
@@ -102,8 +102,9 @@ int __attribute__((target_version("sha2"))) combine(void) { return 1; }
 // expected-error at +1 {{multiversioned function declaration has a different calling convention}}
 int __attribute__((aarch64_vector_pcs, target_version("sha3"))) combine(void) { return 2; }
 
-int __attribute__((target_version("fp+aes+pmull+rcpc"))) unspec_args() { return -1; }
+int unspec_args();
 // expected-error at -1 {{multiversioned function must have a prototype}}
-// expected-error at +1 {{multiversioned function must have a prototype}}
+// expected-note at +1 {{function multiversioning caused by this declaration}}
+int __attribute__((target_version("fp"))) unspec_args() { return -1; }
 int __attribute__((target_version("default"))) unspec_args() { return 0; }
 int cargs() { return unspec_args(); }

>From d5ecade392a1af3cd1b83f4320058330785f1699 Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Fri, 28 Jun 2024 12:05:35 +0100
Subject: [PATCH 2/3] Changes from last revision:

* renamed CheckTargetCausesMultiVersioning to CheckDeclarationCausesMultiVersioning
* broke down the early exit condition in that function as suggested
* removed some more dead code
---
 clang/lib/Sema/SemaDecl.cpp | 35 ++++++++++-------------------------
 1 file changed, 10 insertions(+), 25 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 572c46eed1aaa..890c8671e12b4 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11507,11 +11507,11 @@ static void patchDefaultTargetVersion(FunctionDecl *From, FunctionDecl *To) {
         To->getASTContext(), "default", To->getSourceRange()));
 }
 
-static bool CheckTargetCausesMultiVersioning(Sema &S, FunctionDecl *OldFD,
-                                             FunctionDecl *NewFD,
-                                             bool &Redeclaration,
-                                             NamedDecl *&OldDecl,
-                                             LookupResult &Previous) {
+static bool CheckDeclarationCausesMultiVersioning(Sema &S, FunctionDecl *OldFD,
+                                                  FunctionDecl *NewFD,
+                                                  bool &Redeclaration,
+                                                  NamedDecl *&OldDecl,
+                                                  LookupResult &Previous) {
   assert(!OldFD->isMultiVersion() && "Unexpected MultiVersion");
 
   // The definitions should be allowed in any order. If we have discovered
@@ -11533,8 +11533,9 @@ static bool CheckTargetCausesMultiVersioning(Sema &S, FunctionDecl *OldFD,
   // declaration is NOT the default version. Moreover, the old declaration
   // must be the default version (either explicitly via the attribute,
   // or implicitly without it).
-  if (NewTVA &&
-      (NewTVA->isDefaultVersion() || (OldTVA && !OldTVA->isDefaultVersion())))
+  if (NewTVA && NewTVA->isDefaultVersion())
+    return false;
+  if (NewTVA && OldTVA && !OldTVA->isDefaultVersion())
     return false;
 
   // Otherwise, this decl causes MultiVersioning.
@@ -11583,22 +11584,6 @@ static bool CheckTargetCausesMultiVersioning(Sema &S, FunctionDecl *OldFD,
     }
   }
 
-  if (NewTVA) {
-    llvm::SmallVector<StringRef, 8> Feats;
-    OldTVA->getFeatures(Feats);
-    llvm::sort(Feats);
-    llvm::SmallVector<StringRef, 8> NewFeats;
-    NewTVA->getFeatures(NewFeats);
-    llvm::sort(NewFeats);
-
-    if (Feats == NewFeats) {
-      S.Diag(NewFD->getLocation(), diag::err_multiversion_duplicate);
-      S.Diag(OldFD->getLocation(), diag::note_previous_declaration);
-      NewFD->setInvalidDecl();
-      return true;
-    }
-  }
-
   for (const auto *FD : OldFD->redecls()) {
     const auto *CurTA = FD->getAttr<TargetAttr>();
     const auto *CurTVA = FD->getAttr<TargetVersionAttr>();
@@ -11972,8 +11957,8 @@ static bool CheckMultiVersionFunction(Sema &S, FunctionDecl *NewFD,
     switch (MVKind) {
     case MultiVersionKind::Target:
     case MultiVersionKind::TargetVersion:
-      return CheckTargetCausesMultiVersioning(S, OldFD, NewFD, Redeclaration,
-                                              OldDecl, Previous);
+      return CheckDeclarationCausesMultiVersioning(
+          S, OldFD, NewFD, Redeclaration, OldDecl, Previous);
     case MultiVersionKind::TargetClones:
       if (OldFD->isUsed(false)) {
         NewFD->setInvalidDecl();

>From eaac72a388023331958cb5882409c034db5ac581 Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas <alexandros.lamprineas at arm.com>
Date: Fri, 28 Jun 2024 18:23:33 +0100
Subject: [PATCH 3/3] Changes from last revision:

* removed a redundant early exit condition from CheckDeclarationCausesMultiVersioning
* removed unused variable OldTVA from CheckDeclarationCausesMultiVersioning
* preserved the original test in Sema/attr-target-version.c and added a new one.
---
 clang/lib/Sema/SemaDecl.cpp           |  8 ++------
 clang/test/Sema/attr-target-version.c | 10 +++++++---
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 890c8671e12b4..f8e8c3d5c9f6d 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -11522,7 +11522,7 @@ static bool CheckDeclarationCausesMultiVersioning(Sema &S, FunctionDecl *OldFD,
   const auto *NewTA = NewFD->getAttr<TargetAttr>();
   const auto *NewTVA = NewFD->getAttr<TargetVersionAttr>();
   const auto *OldTA = OldFD->getAttr<TargetAttr>();
-  const auto *OldTVA = OldFD->getAttr<TargetVersionAttr>();
+
   // If the old decl is NOT MultiVersioned yet, and we don't cause that
   // to change, this is a simple redeclaration.
   if (NewTA && !NewTA->isDefaultVersion() &&
@@ -11530,13 +11530,9 @@ static bool CheckDeclarationCausesMultiVersioning(Sema &S, FunctionDecl *OldFD,
     return false;
 
   // The target_version attribute only causes Multiversioning if this
-  // declaration is NOT the default version. Moreover, the old declaration
-  // must be the default version (either explicitly via the attribute,
-  // or implicitly without it).
+  // declaration is NOT the default version.
   if (NewTVA && NewTVA->isDefaultVersion())
     return false;
-  if (NewTVA && OldTVA && !OldTVA->isDefaultVersion())
-    return false;
 
   // Otherwise, this decl causes MultiVersioning.
   if (CheckMultiVersionAdditionalRules(S, OldFD, NewFD, true,
diff --git a/clang/test/Sema/attr-target-version.c b/clang/test/Sema/attr-target-version.c
index 8de3eeeaff8f5..88a927a58f991 100644
--- a/clang/test/Sema/attr-target-version.c
+++ b/clang/test/Sema/attr-target-version.c
@@ -102,9 +102,13 @@ int __attribute__((target_version("sha2"))) combine(void) { return 1; }
 // expected-error at +1 {{multiversioned function declaration has a different calling convention}}
 int __attribute__((aarch64_vector_pcs, target_version("sha3"))) combine(void) { return 2; }
 
-int unspec_args();
+int __attribute__((target_version("fp+aes+pmull+rcpc"))) unspec_args() { return -1; }
 // expected-error at -1 {{multiversioned function must have a prototype}}
-// expected-note at +1 {{function multiversioning caused by this declaration}}
-int __attribute__((target_version("fp"))) unspec_args() { return -1; }
 int __attribute__((target_version("default"))) unspec_args() { return 0; }
 int cargs() { return unspec_args(); }
+
+int unspec_args_implicit_default_first();
+// expected-error at -1 {{multiversioned function must have a prototype}}
+// 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; }



More information about the cfe-commits mailing list