Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

[RISCV][FMV] Support target_version #99040

Merged
merged 13 commits into from
Oct 4, 2024
Merged

[RISCV][FMV] Support target_version #99040

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
54b5d68
[RISCV][FMV] Support target_version
BeMg Mar 19, 2024
68cbc16
Use single quote for char
BeMg Sep 13, 2024
c7ad1a9
Refine comment
BeMg Sep 13, 2024
5414680
Update attr-target-version-riscv-invalid.c
BeMg Sep 18, 2024
7c6f108
Reduce some duplication
BeMg Sep 18, 2024
5e00115
Add assert for Aarch64 target
BeMg Sep 23, 2024
2fcfc9c
isPriority -> hasPriority
BeMg Sep 23, 2024
a487f19
Guard the default with arch
BeMg Sep 23, 2024
b89211c
Guard hasFeature("fmv") with getTriple().isAArch64()
BeMg Sep 23, 2024
8cd911d
Drop blank line
BeMg Sep 24, 2024
e98d84f
Add assert
BeMg Sep 24, 2024
e449161
IsDefault -> HasDefault
BeMg Sep 24, 2024
9c70204
Warning more invalid pattern
BeMg Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 11 additions & 3 deletions clang/lib/AST/ASTContext.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14270,9 +14270,17 @@ void ASTContext::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU, Features);
}
} else if (const auto *TV = FD->getAttr<TargetVersionAttr>()) {
llvm::SmallVector<StringRef, 8> Feats;
TV->getFeatures(Feats);
std::vector<std::string> Features = getFMVBackendFeaturesFor(Feats);
std::vector<std::string> Features;
if (Target->getTriple().isRISCV()) {
ParsedTargetAttr ParsedAttr = Target->parseTargetAttr(TV->getName());
Features.insert(Features.begin(), ParsedAttr.Features.begin(),
ParsedAttr.Features.end());
} else {
assert(Target->getTriple().isAArch64());
llvm::SmallVector<StringRef, 8> Feats;
BeMg marked this conversation as resolved.
Show resolved Hide resolved
TV->getFeatures(Feats);
Features = getFMVBackendFeaturesFor(Feats);
}
Features.insert(Features.begin(),
Target->getTargetOpts().FeaturesAsWritten.begin(),
Target->getTargetOpts().FeaturesAsWritten.end());
Expand Down
7 changes: 6 additions & 1 deletion clang/lib/CodeGen/CodeGenModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4268,8 +4268,13 @@ void CodeGenModule::emitMultiVersionFunctions() {
} else if (const auto *TVA = CurFD->getAttr<TargetVersionAttr>()) {
if (TVA->isDefaultVersion() && IsDefined)
ShouldEmitResolver = true;
TVA->getFeatures(Feats);
llvm::Function *Func = createFunction(CurFD);
if (getTarget().getTriple().isRISCV()) {
Feats.push_back(TVA->getName());
} else {
assert(getTarget().getTriple().isAArch64());
TVA->getFeatures(Feats);
BeMg marked this conversation as resolved.
Show resolved Hide resolved
}
Options.emplace_back(Func, /*Architecture*/ "", Feats);
} else if (const auto *TC = CurFD->getAttr<TargetClonesAttr>()) {
if (IsDefined)
Expand Down
19 changes: 15 additions & 4 deletions clang/lib/Sema/SemaDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10319,7 +10319,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
// Handle attributes.
ProcessDeclAttributes(S, NewFD, D);
const auto *NewTVA = NewFD->getAttr<TargetVersionAttr>();
if (NewTVA && !NewTVA->isDefaultVersion() &&
if (Context.getTargetInfo().getTriple().isAArch64() && NewTVA &&
!NewTVA->isDefaultVersion() &&
!Context.getTargetInfo().hasFeature("fmv")) {
// Don't add to scope fmv functions declarations if fmv disabled
AddToScope = false;
Expand Down Expand Up @@ -11028,7 +11029,15 @@ static bool CheckMultiVersionValue(Sema &S, const FunctionDecl *FD) {

if (TVA) {
llvm::SmallVector<StringRef, 8> Feats;
TVA->getFeatures(Feats);
if (S.getASTContext().getTargetInfo().getTriple().isRISCV()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The need to version all of this by target is unfortunate. Is there a way we can reduce the duplication here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I reduce the part of duplication, and the remain part is causing by different syntax between aarch64 and RISC-V.

To reducing the remain part of duplication, we could implement the RISC-V syntax inside clang/include/clang/Basic/Attr.td getFeatures or make Aarch64 using the AArch64TargetInfo::parseTargetAttr for target_version, or make something like parseTargetVersionAttr hook here.

By the way, the parsing of target_clones syntax is controlled by the target. (Reference:

llvm-project/clang/lib/Sema/SemaDeclAttr.cpp

Line 3121 in 9cd9377

if (TInfo.getTriple().isAArch64()) {
)

ParsedTargetAttr ParseInfo =
S.getASTContext().getTargetInfo().parseTargetAttr(TVA->getName());
for (auto &Feat : ParseInfo.Features)
Feats.push_back(StringRef{Feat}.substr(1));
} else {
assert(S.getASTContext().getTargetInfo().getTriple().isAArch64());
TVA->getFeatures(Feats);
BeMg marked this conversation as resolved.
Show resolved Hide resolved
}
for (const auto &Feat : Feats) {
if (!TargetInfo.validateCpuSupports(Feat)) {
S.Diag(FD->getLocation(), diag::err_bad_multiversion_option)
Expand Down Expand Up @@ -11314,7 +11323,8 @@ static bool PreviousDeclsHaveMultiVersionAttribute(const FunctionDecl *FD) {
}

static void patchDefaultTargetVersion(FunctionDecl *From, FunctionDecl *To) {
if (!From->getASTContext().getTargetInfo().getTriple().isAArch64())
if (!From->getASTContext().getTargetInfo().getTriple().isAArch64() &&
!From->getASTContext().getTargetInfo().getTriple().isRISCV())
return;

MultiVersionKind MVKindFrom = From->getMultiVersionKind();
Expand Down Expand Up @@ -15501,7 +15511,8 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
FD->setInvalidDecl();
}
if (const auto *Attr = FD->getAttr<TargetVersionAttr>()) {
if (!Context.getTargetInfo().hasFeature("fmv") &&
if (Context.getTargetInfo().getTriple().isAArch64() &&
!Context.getTargetInfo().hasFeature("fmv") &&
!Attr->isDefaultVersion()) {
// If function multi versioning disabled skip parsing function body
// defined with non-default target_version attribute
Expand Down
48 changes: 48 additions & 0 deletions clang/lib/Sema/SemaDeclAttr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3056,6 +3056,54 @@ bool Sema::checkTargetVersionAttr(SourceLocation LiteralLoc, Decl *D,
enum SecondParam { None };
enum ThirdParam { Target, TargetClones, TargetVersion };
llvm::SmallVector<StringRef, 8> Features;
if (Context.getTargetInfo().getTriple().isRISCV()) {
llvm::SmallVector<StringRef, 8> AttrStrs;
AttrStr.split(AttrStrs, ';');

bool HasArch = false;
bool HasPriority = false;
bool HasDefault = false;
bool DuplicateAttr = false;
for (auto &AttrStr : AttrStrs) {
// Only support arch=+ext,... syntax.
topperc marked this conversation as resolved.
Show resolved Hide resolved
if (AttrStr.starts_with("arch=+")) {
if (HasArch)
DuplicateAttr = true;
HasArch = true;
ParsedTargetAttr TargetAttr =
Context.getTargetInfo().parseTargetAttr(AttrStr);

if (TargetAttr.Features.empty() ||
llvm::any_of(TargetAttr.Features, [&](const StringRef Ext) {
return !RISCV().isValidFMVExtension(Ext);
}))
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
<< Unsupported << None << AttrStr << TargetVersion;
} else if (AttrStr.starts_with("default")) {
if (HasDefault)
DuplicateAttr = true;
HasDefault = true;
} else if (AttrStr.consume_front("priority=")) {
if (HasPriority)
DuplicateAttr = true;
HasPriority = true;
int Digit;
if (AttrStr.getAsInteger(0, Digit))
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
<< Unsupported << None << AttrStr << TargetVersion;
} else {
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
<< Unsupported << None << AttrStr << TargetVersion;
}
}

if (((HasPriority || HasArch) && HasDefault) || DuplicateAttr ||
(HasPriority && !HasArch))
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
<< Unsupported << None << AttrStr << TargetVersion;

return false;
}
AttrStr.split(Features, "+");
for (auto &CurFeature : Features) {
CurFeature = CurFeature.trim();
Expand Down
13 changes: 13 additions & 0 deletions clang/test/CodeGen/attr-target-version-riscv-invalid.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// RUN: not %clang_cc1 -triple riscv64 -target-feature +i -emit-llvm -o - %s 2>&1 | FileCheck %s --check-prefix=CHECK-UNSUPPORT-OS

// CHECK-UNSUPPORT-OS: error: function multiversioning is currently only supported on Linux
__attribute__((target_version("default"))) int foo(void) {
return 2;
}

__attribute__((target_version("arch=+c"))) int foo(void) {
return 2;
}


int bar() { return foo(); }
Loading