[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `sanitizer-ppc64be-linux` running on `ppc64be-sanitizer` while building `clang` at step 2 "annotate". Full details are available at: https://lab.llvm.org/buildbot/#/builders/109/builds/643 Here is the relevant piece of the build log for the reference: ``` Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure) ... PASS: Profile-powerpc64 :: Linux/instrprof-dlopen.test (388 of 2421) PASS: ThreadSanitizer-powerpc64 :: fd_location.cpp (389 of 2421) PASS: SanitizerCommon-tsan-powerpc64-Linux :: Posix/mmap_write_exec.cpp (390 of 2421) XFAIL: SanitizerCommon-tsan-powerpc64-Linux :: Linux/allow_user_segv.cpp (391 of 2421) XFAIL: SanitizerCommon-tsan-powerpc64-Linux :: Posix/dump_registers.cpp (392 of 2421) PASS: ThreadSanitizer-powerpc64 :: java_finalizer2.cpp (393 of 2421) PASS: SanitizerCommon-msan-powerpc64-Linux :: Posix/sanitizer_set_report_fd_test.cpp (394 of 2421) PASS: ThreadSanitizer-powerpc64 :: cond_destruction.cpp (395 of 2421) XFAIL: SanitizerCommon-tsan-powerpc64-Linux :: Posix/sanitizer_set_report_fd_test.cpp (396 of 2421) PASS: ThreadSanitizer-powerpc64 :: pthread_atfork_deadlock2.c (397 of 2421) FAIL: ThreadSanitizer-powerpc64 :: signal_block.cpp (398 of 2421) TEST 'ThreadSanitizer-powerpc64 :: signal_block.cpp' FAILED Exit Code: 1 Command Output (stderr): -- RUN: at line 1: /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/./bin/clang -fsanitize=thread -Wall -m64 -gline-tables-only -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/../ -O1 /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_block.cpp -o /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_block.cpp.tmp && /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_block.cpp.tmp 2>&1 | FileCheck /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_block.cpp + /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/./bin/clang -fsanitize=thread -Wall -m64 -gline-tables-only -I/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/../ -O1 /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_block.cpp -o /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_block.cpp.tmp + /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_block.cpp.tmp + FileCheck /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_block.cpp /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_block.cpp:59:15: error: CHECK-NOT: excluded string found in input // CHECK-NOT: WARNING: ThreadSanitizer: ^ :2:1: note: found here WARNING: ThreadSanitizer: signal handler spoils errno (pid=134856) ^ Input file: Check file: /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_block.cpp -dump-input=help explains the following input dump. Input was: << 1: == 2: WARNING: ThreadSanitizer: signal handler spoils errno (pid=134856) not:59 ! error: no match expected 3: Signal 10 handler invoked at: 4: #0 _DYNAMIC (signal_block.cpp.tmp+0x16fe68) 5: #1 thread(void*) /home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/compiler-rt/test/tsan/signal_block.cpp:25:5 (signal_block.cpp.tmp+0x11cf8c) 6: 7: SUMMARY: ThreadSanitizer: signal handler spoils errno (/home/buildbots/llvm-external-buildbots/workers/ppc64be-sanitizer/sanitizer-ppc64be/build/build_debug/runtimes/runtimes-bins/compiler-rt/test/tsan/POWERPC64Config/Output/signal_block.cpp.tmp+0x16fe68) in _DYNAMIC 8: == 9: DONE 10:
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
llvm-ci wrote: LLVM Buildbot has detected a new failure on builder `lldb-x86_64-debian` running on `lldb-x86_64-debian` while building `clang` at step 6 "test". Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/1430 Here is the relevant piece of the build log for the reference: ``` Step 6 (test) failure: build (failure) ... PASS: lldb-api :: macosx/load-kext/TestLoadKext.py (1080 of 2628) UNSUPPORTED: lldb-api :: python_api/target-arch-from-module/TestTargetArchFromModule.py (1081 of 2628) UNSUPPORTED: lldb-api :: lang/objc/global_ptrs/TestGlobalObjects.py (1082 of 2628) PASS: lldb-api :: commands/version/TestVersion.py (1083 of 2628) UNSUPPORTED: lldb-api :: lang/objc/modules-inline-functions/TestModulesInlineFunctions.py (1084 of 2628) UNSUPPORTED: lldb-api :: lang/objc/foundation/TestObjCMethods.py (1085 of 2628) UNSUPPORTED: lldb-api :: functionalities/type_lookup/TestTypeLookup.py (1086 of 2628) UNSUPPORTED: lldb-api :: macosx/stack-corefile/TestStackCorefile.py (1087 of 2628) UNSUPPORTED: lldb-api :: functionalities/interactive_scripted_process/TestInteractiveScriptedProcess.py (1088 of 2628) UNSUPPORTED: lldb-api :: commands/expression/weak_symbols/TestWeakSymbols.py (1089 of 2628) FAIL: lldb-shell :: SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp (1090 of 2628) TEST 'lldb-shell :: SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp' FAILED Exit Code: 1 Command Output (stderr): -- RUN: at line 21: /home/worker/2.0.1/lldb-x86_64-debian/build/bin/clang --target=specify-a-target-or-use-a-_host-substitution -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf-fdebug-types-section -gpubnames -c /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp -o /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/dwp-foreign-type-units.cpp.tmp.main.o + /home/worker/2.0.1/lldb-x86_64-debian/build/bin/clang --target=specify-a-target-or-use-a-_host-substitution -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -fdebug-types-section -gpubnames -c /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp -o /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/dwp-foreign-type-units.cpp.tmp.main.o RUN: at line 23: /home/worker/2.0.1/lldb-x86_64-debian/build/bin/clang --target=specify-a-target-or-use-a-_host-substitution -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -DVARIANT-fdebug-types-section -gpubnames -c /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp -o /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/dwp-foreign-type-units.cpp.tmp.foo.o + /home/worker/2.0.1/lldb-x86_64-debian/build/bin/clang --target=specify-a-target-or-use-a-_host-substitution -target x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -DVARIANT -fdebug-types-section -gpubnames -c /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-foreign-type-units.cpp -o /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/dwp-foreign-type-units.cpp.tmp.foo.o RUN: at line 25: /home/worker/2.0.1/lldb-x86_64-debian/build/bin/ld.lld /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/dwp-foreign-type-units.cpp.tmp.main.o /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/dwp-foreign-type-units.cpp.tmp.foo.o -o /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/dwp-foreign-type-units.cpp.tmp + /home/worker/2.0.1/lldb-x86_64-debian/build/bin/ld.lld /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/dwp-foreign-type-units.cpp.tmp.main.o /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/dwp-foreign-type-units.cpp.tmp.foo.o -o /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/dwp-foreign-type-units.cpp.tmp ld.lld: warning: cannot find entry symbol _start; not setting start address RUN: at line 28: rm -f /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/dwp-foreign-type-units.cpp.tmp.dwp + rm -f /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/SymbolFile/DWARF/x86/Output/dwp-foreign-type-units.cpp.tmp.dwp RUN: at line 29: /home/worker/2.0.1/lldb-x86_64-debian/build/bin/lldb --no-lldbinit -S /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb/test/Shell/lit-lldb-init-quiet -o "type lookup IntegerType"-o "type lookup FloatType"-o "type lookup CustomType"-b
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/labrinea closed https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
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
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 , bool );
+ bool checkTargetVersionAttr(SourceLocation Loc, Decl *D, StringRef Str);
bool checkTargetClonesAttrString(
SourceLocation LiteralLoc, StringRef Str, const StringLiteral *Literal,
Decl *D, bool , bool , bool ,
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 ,
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 , FunctionDecl *OldFD,
@@ -11523,10 +11525,16 @@ static bool CheckTargetCausesMultiVersioning(Sema ,
FunctionDecl *OldFD,
const auto *OldTVA = OldFD->getAttr();
// 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 ,
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 ,
FunctionDecl *NewFD,
FunctionDecl *OldFD = OldDecl->getAsFunction();
- if (!OldFD->isMultiVersion() && MVKind == MultiVersionKind::None) {
-if (NewTVA || !OldFD->getAttr())
- return false;
-if (!NewFD->getType()->getAs()) {
- // Multiversion declaration doesn't have prototype.
-
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/ilinpv approved this pull request. Look good, thanks for refactoring and removing obsolete checks after FMV changes related to ordering and default version. https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
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
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 , bool );
+ bool checkTargetVersionAttr(SourceLocation Loc, Decl *D, StringRef Str);
bool checkTargetClonesAttrString(
SourceLocation LiteralLoc, StringRef Str, const StringLiteral *Literal,
Decl *D, bool , bool , bool ,
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 ,
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 , FunctionDecl *OldFD,
@@ -11523,10 +11525,16 @@ static bool CheckTargetCausesMultiVersioning(Sema ,
FunctionDecl *OldFD,
const auto *OldTVA = OldFD->getAttr();
// 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 ,
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 ,
FunctionDecl *NewFD,
FunctionDecl *OldFD = OldDecl->getAsFunction();
- if (!OldFD->isMultiVersion() && MVKind == MultiVersionKind::None) {
-if (NewTVA || !OldFD->getAttr())
- return false;
-if (!NewFD->getType()->getAs()) {
- // Multiversion declaration doesn't have prototype.
-
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
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
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 , bool );
+ bool checkTargetVersionAttr(SourceLocation Loc, Decl *D, StringRef Str);
bool checkTargetClonesAttrString(
SourceLocation LiteralLoc, StringRef Str, const StringLiteral *Literal,
Decl *D, bool , bool , bool ,
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 ,
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 , FunctionDecl *OldFD,
@@ -11523,10 +11525,16 @@ static bool CheckTargetCausesMultiVersioning(Sema ,
FunctionDecl *OldFD,
const auto *OldTVA = OldFD->getAttr();
// 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 ,
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 ,
FunctionDecl *NewFD,
FunctionDecl *OldFD = OldDecl->getAsFunction();
- if (!OldFD->isMultiVersion() && MVKind == MultiVersionKind::None) {
-if (NewTVA || !OldFD->getAttr())
- return false;
-if (!NewFD->getType()->getAs()) {
- // Multiversion declaration doesn't have prototype.
-
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/tmatheson-arm approved this pull request. https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/labrinea edited https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/labrinea edited https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
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
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 , bool );
+ bool checkTargetVersionAttr(SourceLocation Loc, Decl *D, StringRef Str);
bool checkTargetClonesAttrString(
SourceLocation LiteralLoc, StringRef Str, const StringLiteral *Literal,
Decl *D, bool , bool , bool ,
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 ,
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 , FunctionDecl *OldFD,
@@ -11523,10 +11525,16 @@ static bool CheckTargetCausesMultiVersioning(Sema ,
FunctionDecl *OldFD,
const auto *OldTVA = OldFD->getAttr();
// 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 ,
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 ,
FunctionDecl *NewFD,
FunctionDecl *OldFD = OldDecl->getAsFunction();
- if (!OldFD->isMultiVersion() && MVKind == MultiVersionKind::None) {
-if (NewTVA || !OldFD->getAttr())
- return false;
-if (!NewFD->getType()->getAs()) {
- // Multiversion declaration doesn't have prototype.
-
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11465,6 +11465,10 @@ static bool CheckMultiVersionFirstFunction(Sema ,
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;
labrinea wrote:
`error: 'target_version' and 'target' attributes are not compatible`
No it's not allowed, but that doesn't mean that the early exit conditions
cannot interfere. For example as I explain below !TVA should not early exit
because this function check the target attribute as well.
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/labrinea edited https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11576,22 +11584,6 @@ static bool CheckTargetCausesMultiVersioning(Sema ,
FunctionDecl *OldFD,
}
}
- if (NewTVA) {
labrinea wrote:
The assertion should be conditional when NewTVA is not null because the
function is used for the target attribute as well. Do we still want the
assertion?
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/labrinea edited https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11523,10 +11525,17 @@ static bool CheckTargetCausesMultiVersioning(Sema , FunctionDecl *OldFD, const auto *OldTVA = OldFD->getAttr(); // 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()) +return false; labrinea wrote: same here, if we shouldn't eraly exist if NewTVA is null. https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11465,6 +11465,10 @@ static bool CheckMultiVersionFirstFunction(Sema ,
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;
labrinea wrote:
Hmm, we can't write it this way I am affraid because the function used both for
target and target_version attributes. If TA is not null and TVA is null we are
early exiting while we shouldn't.
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/tmatheson-arm edited https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -102,8 +102,9 @@ int __attribute__((target_version("sha2"))) combine(void) {
return 1; }
// expected-error@+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@-1 {{multiversioned function must have a prototype}}
-// expected-error@+1 {{multiversioned function must have a prototype}}
+// expected-note@+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; }
tmatheson-arm wrote:
Either way lets keep the original test, if only to document what that case does
now.
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -102,8 +102,9 @@ int __attribute__((target_version("sha2"))) combine(void) {
return 1; }
// expected-error@+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@-1 {{multiversioned function must have a prototype}}
-// expected-error@+1 {{multiversioned function must have a prototype}}
+// expected-note@+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; }
tmatheson-arm wrote:
Can we keep the test to check that we _don't_ emit a diagnostic then?
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/tmatheson-arm deleted https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11465,6 +11465,10 @@ static bool CheckMultiVersionFirstFunction(Sema ,
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;
tmatheson-arm wrote:
```suggestion
if (!TVA || TVA->isDefaultVersion())
return false;
```
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11947,24 +11939,8 @@ static bool CheckMultiVersionFunction(Sema ,
FunctionDecl *NewFD,
FunctionDecl *OldFD = OldDecl->getAsFunction();
- if (!OldFD->isMultiVersion() && MVKind == MultiVersionKind::None) {
-if (NewTVA || !OldFD->getAttr())
- return false;
-if (!NewFD->getType()->getAs()) {
- // 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)
tmatheson-arm wrote:
`!OldFD->isMultiVersion()` is asserted at the top of the function, so this
looks unnecessary. However OldFD is redefined here. This is confusing. Does the
original assert still hold with the new assignment?
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11523,10 +11525,17 @@ static bool CheckTargetCausesMultiVersioning(Sema , FunctionDecl *OldFD, const auto *OldTVA = OldFD->getAttr(); // 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()) +return false; + if (NewTVA && OldTVA && !OldTVA->isDefaultVersion()) return false; tmatheson-arm wrote: ```suggestion ``` We already know that `!isMultiVersioned()` at this point, because of the call site (and assert at the top of the function). Presumably these checks for the old version have already been done? https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11523,10 +11525,17 @@ static bool CheckTargetCausesMultiVersioning(Sema ,
FunctionDecl *OldFD,
const auto *OldTVA = OldFD->getAttr();
// 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())
+return false;
tmatheson-arm wrote:
```suggestion
// If this is target_version("default") (implicit or explicit) it doesn't
trigger MV
if (!NewTVA || NewTVA->isDefaultVersion())
return false;
```
This is my understanding of what we want to do here.
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11576,22 +11584,6 @@ static bool CheckTargetCausesMultiVersioning(Sema ,
FunctionDecl *OldFD,
}
}
- if (NewTVA) {
tmatheson-arm wrote:
If you are reasoning based on that, it might be good to add an assert checking
it:
```
assert(!OldTVA || OldTVA->isDefaultVersion());
```
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11465,6 +11465,10 @@ static bool CheckMultiVersionFirstFunction(Sema ,
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;
tmatheson-arm wrote:
Is it allowed to have both `target` and `target_version` on the same function?
If so, can these early returns interfere with each other?
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/labrinea edited https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/labrinea edited https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11576,22 +11584,6 @@ static bool CheckTargetCausesMultiVersioning(Sema ,
FunctionDecl *OldFD,
}
}
- if (NewTVA) {
labrinea wrote:
This is checking things like
```
//expected-note@+1 {{previous declaration is here}}
void __attribute__((target_version("bti+flagm2"))) one(void) {}
//expected-error@+1 {{multiversioned function redeclarations require identical
target attributes}}
void __attribute__((target_version("flagm2+bti"))) one(void) {}
```
(example taken from clang/test/Sema/attr-target-version.c)
At this point we know that the new declaration is triggering multiversioning
therefore the old was the (implicit or explicit) default.
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
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
Date: Tue, 25 Jun 2024 00:27:45 +0100
Subject: [PATCH 1/2] [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 , bool );
+ bool checkTargetVersionAttr(SourceLocation Loc, Decl *D, StringRef Str);
bool checkTargetClonesAttrString(
SourceLocation LiteralLoc, StringRef Str, const StringLiteral *Literal,
Decl *D, bool , bool , bool ,
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 ,
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 , FunctionDecl *OldFD,
@@ -11523,10 +11525,16 @@ static bool CheckTargetCausesMultiVersioning(Sema ,
FunctionDecl *OldFD,
const auto *OldTVA = OldFD->getAttr();
// 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 ,
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 ,
FunctionDecl *NewFD,
FunctionDecl *OldFD = OldDecl->getAsFunction();
- if (!OldFD->isMultiVersion() && MVKind == MultiVersionKind::None) {
-if (NewTVA || !OldFD->getAttr())
- return false;
-if (!NewFD->getType()->getAs()) {
- // Multiversion declaration doesn't have prototype.
-
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11523,10 +11525,16 @@ static bool CheckTargetCausesMultiVersioning(Sema , FunctionDecl *OldFD, const auto *OldTVA = OldFD->getAttr(); // 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. labrinea wrote: Which conditions? I think at this point we have got passed the early exit conditions which signify not causing multiversioning. https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/labrinea edited https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/labrinea edited https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/labrinea edited https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -102,8 +102,9 @@ int __attribute__((target_version("sha2"))) combine(void) {
return 1; }
// expected-error@+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@-1 {{multiversioned function must have a prototype}}
labrinea wrote:
The C99 standards says
"A function prototype is a declaration of a function that declares the types of
its parameters".
"An identifier list in a function declarator that is not part of a definition
of that function shall be empty."
"After adjustment, the parameters in a parameter type list in a function
declarator that is part of a definition of that function shall not have
incomplete type."
"A parameter type list specifies the types of, and may declare identifiers for,
the parameters of the function."
"The special case of an unnamed parameter of type void as the only item in the
list specifies that the function has no parameters."
"An identifier list declares only the identifiers of the parameters of the
function. An empty list in a function declarator that is part of a definition
of that function specifies that the function has no parameters. **The empty
list in a function declarator that is not part of a definition of that function
specifies that no information about the number or types of the parameters is
supplied**".
In our case the declarator is part of a definition as far as I understand.
The latest C standard I found says
"For a function declarator without a parameter type list: the effect is as if
it were declared with a parameter type list consisting of the keyword void. A
function declarator provides a prototype for the function."
So perhaps we don't want to diagnose this case at all. Anyhow I don't think
such a change should be part of this patch.
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -102,8 +102,9 @@ int __attribute__((target_version("sha2"))) combine(void) {
return 1; }
// expected-error@+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@-1 {{multiversioned function must have a prototype}}
-// expected-error@+1 {{multiversioned function must have a prototype}}
+// expected-note@+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; }
labrinea wrote:
Yes. We are no longer diagnosing `__attribute__((target_version("default")))
unspec_args()` because it does not trigger multiversioning.
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -102,8 +102,9 @@ int __attribute__((target_version("sha2"))) combine(void) {
return 1; }
// expected-error@+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@-1 {{multiversioned function must have a prototype}}
tmatheson-arm wrote:
This is not the most helpful message. It doesn't have a prototype because
`void` is missing from the argument list?
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -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 , FunctionDecl *OldFD,
tmatheson-arm wrote:
```suggestion
static bool CheckAttributeCausesMultiVersioning(Sema , FunctionDecl *OldFD,
```
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11523,10 +11525,16 @@ static bool CheckTargetCausesMultiVersioning(Sema , FunctionDecl *OldFD, const auto *OldTVA = OldFD->getAttr(); // 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. tmatheson-arm wrote: This comment is out of date, since there are more conditions below it. https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -102,8 +102,9 @@ int __attribute__((target_version("sha2"))) combine(void) {
return 1; }
// expected-error@+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@-1 {{multiversioned function must have a prototype}}
-// expected-error@+1 {{multiversioned function must have a prototype}}
+// expected-note@+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; }
tmatheson-arm wrote:
Is there a reason not to keep the original test too?
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11523,10 +11525,16 @@ static bool CheckTargetCausesMultiVersioning(Sema , FunctionDecl *OldFD, const auto *OldTVA = OldFD->getAttr(); // 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; tmatheson-arm wrote: ```suggestion if (NewTVA && NewTVA->isDefaultVersion()) return false; if (NewTVA && OldTVA && !OldTVA->isDefaultVersion()) return false; ``` https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/labrinea edited https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/Endilll commented: `Sema.h` changes look good to me. https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/Endilll edited https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
@@ -11543,8 +11551,7 @@ static bool CheckTargetCausesMultiVersioning(Sema ,
FunctionDecl *OldFD,
}
// If this is 'default', permit the forward declaration.
- if ((NewTA && NewTA->isDefaultVersion() && !OldTA) ||
- (NewTVA && NewTVA->isDefaultVersion() && !OldTVA)) {
labrinea wrote:
This is wrong. The new declaration cannot be a redeclaration of the old, and at
the same time to be causing Multiversioning. Getting past the early return
(line 11538 with the new code) means the new declaration is causing
Multiversioning, so at this point (line 11554 with the new code) redeclaration
is not possible for the target_version attribute.
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
labrinea wrote: @ilinpv, @DanielKristofKiss, the patch https://github.com/llvm/llvm-project/pull/65671/files doesn't seem right. I have basically reverted it with my change, ensuring the original crash you were trying to fix doesn't occur. FYI `MVKind == MultiVersionKind::None` is equivallent to `NewTVA == nullptr` in that context. https://github.com/llvm/llvm-project/pull/96628 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
labrinea wrote:
Alternative one liner:
```
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp
b/clang/lib/CodeGen/CodeGenModule.cpp
index dd4a665ebc78..0c96a6de091a 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3763,7 +3763,7 @@ void CodeGenModule::EmitGlobal(GlobalDecl GD) {
// Forward declarations are emitted lazily on first use.
if (!FD->doesThisDeclarationHaveABody()) {
if (!FD->doesDeclarationForceExternallyVisibleDefinition() &&
- (!FD->isMultiVersion() ||
+ (FD->getMultiVersionKind() == MultiVersionKind::None ||
!FD->getASTContext().getTargetInfo().getTriple().isAArch64()))
return;
```
but I believe not emitting the explicit default and then patching it later is a
hack.
https://github.com/llvm/llvm-project/pull/96628
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: Alexandros Lamprineas (labrinea)
Changes
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
---
Full diff: https://github.com/llvm/llvm-project/pull/96628.diff
6 Files Affected:
- (modified) clang/include/clang/Sema/Sema.h (+1-2)
- (modified) clang/lib/Sema/SemaDecl.cpp (+18-27)
- (modified) clang/lib/Sema/SemaDeclAttr.cpp (+5-11)
- (modified) clang/test/CodeGen/attr-target-version.c (+44-44)
- (modified) clang/test/CodeGenCXX/fmv-namespace.cpp (+47-13)
- (modified) clang/test/Sema/attr-target-version.c (+3-2)
``diff
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 , bool );
+ bool checkTargetVersionAttr(SourceLocation Loc, Decl *D, StringRef Str);
bool checkTargetClonesAttrString(
SourceLocation LiteralLoc, StringRef Str, const StringLiteral *Literal,
Decl *D, bool , bool , bool ,
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 ,
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 , FunctionDecl *OldFD,
@@ -11523,10 +11525,16 @@ static bool CheckTargetCausesMultiVersioning(Sema ,
FunctionDecl *OldFD,
const auto *OldTVA = OldFD->getAttr();
// 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 ,
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 ,
FunctionDecl *NewFD,
FunctionDecl *OldFD = OldDecl->getAsFunction();
- if (!OldFD->isMultiVersion() && MVKind == MultiVersionKind::None) {
-if (NewTVA || !OldFD->getAttr())
- return false;
-if (!NewFD->getType()->getAs()) {
- // 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.
[clang] [clang][FMV] Do not omit explicit default target_version attribute. (PR #96628)
https://github.com/labrinea created
https://github.com/llvm/llvm-project/pull/96628
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
>From ff4635208e9cd83c6735c95ebf12125ca737029a Mon Sep 17 00:00:00 2001
From: Alexandros Lamprineas
Date: Tue, 25 Jun 2024 00:27:45 +0100
Subject: [PATCH] [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 , bool );
+ bool checkTargetVersionAttr(SourceLocation Loc, Decl *D, StringRef Str);
bool checkTargetClonesAttrString(
SourceLocation LiteralLoc, StringRef Str, const StringLiteral *Literal,
Decl *D, bool , bool , bool ,
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 ,
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 , FunctionDecl *OldFD,
@@ -11523,10 +11525,16 @@ static bool CheckTargetCausesMultiVersioning(Sema ,
FunctionDecl *OldFD,
const auto *OldTVA = OldFD->getAttr();
// 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 ,
FunctionDecl *OldFD,
}
// If this is 'default', permit the forward declaration.
- if ((NewTA && NewTA->isDefaultVersion() && !OldTA) ||
- (NewTVA && NewTVA->isDefaultVersion() && !OldTVA)) {
+ if (NewTA && NewTA->isDefaultVersion() &&
