Skip to content

Commit f658c1b

Browse files
authored
Recommit "[RISCV][FMV] Support target_version" (#111096)" (#111333)
Fix the buildbot failure caused by heap use-after-free error. Origin message: This patch enable `target_version` attribute for RISC-V target. The proposal of `target_version` syntax can be found at the riscv-non-isa/riscv-c-api-doc#48 (which has landed), as modified by the proposed riscv-non-isa/riscv-c-api-doc#85 (which adds the priority syntax). `target_version` attribute will trigger the function multi-versioning feature and act like `target_clones` attribute. See #85786 for the implementation of `target_clones`.
1 parent 634c57d commit f658c1b

File tree

8 files changed

+1082
-8
lines changed

8 files changed

+1082
-8
lines changed

clang/lib/AST/ASTContext.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14325,9 +14325,17 @@ void ASTContext::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
1432514325
Target->initFeatureMap(FeatureMap, getDiagnostics(), TargetCPU, Features);
1432614326
}
1432714327
} else if (const auto *TV = FD->getAttr<TargetVersionAttr>()) {
14328-
llvm::SmallVector<StringRef, 8> Feats;
14329-
TV->getFeatures(Feats);
14330-
std::vector<std::string> Features = getFMVBackendFeaturesFor(Feats);
14328+
std::vector<std::string> Features;
14329+
if (Target->getTriple().isRISCV()) {
14330+
ParsedTargetAttr ParsedAttr = Target->parseTargetAttr(TV->getName());
14331+
Features.insert(Features.begin(), ParsedAttr.Features.begin(),
14332+
ParsedAttr.Features.end());
14333+
} else {
14334+
assert(Target->getTriple().isAArch64());
14335+
llvm::SmallVector<StringRef, 8> Feats;
14336+
TV->getFeatures(Feats);
14337+
Features = getFMVBackendFeaturesFor(Feats);
14338+
}
1433114339
Features.insert(Features.begin(),
1433214340
Target->getTargetOpts().FeaturesAsWritten.begin(),
1433314341
Target->getTargetOpts().FeaturesAsWritten.end());

clang/lib/CodeGen/CodeGenModule.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4287,8 +4287,13 @@ void CodeGenModule::emitMultiVersionFunctions() {
42874287
} else if (const auto *TVA = CurFD->getAttr<TargetVersionAttr>()) {
42884288
if (TVA->isDefaultVersion() && IsDefined)
42894289
ShouldEmitResolver = true;
4290-
TVA->getFeatures(Feats);
42914290
llvm::Function *Func = createFunction(CurFD);
4291+
if (getTarget().getTriple().isRISCV()) {
4292+
Feats.push_back(TVA->getName());
4293+
} else {
4294+
assert(getTarget().getTriple().isAArch64());
4295+
TVA->getFeatures(Feats);
4296+
}
42924297
Options.emplace_back(Func, /*Architecture*/ "", Feats);
42934298
} else if (const auto *TC = CurFD->getAttr<TargetClonesAttr>()) {
42944299
if (IsDefined)

clang/lib/Sema/SemaDecl.cpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10329,7 +10329,8 @@ Sema::ActOnFunctionDeclarator(Scope *S, Declarator &D, DeclContext *DC,
1032910329
// Handle attributes.
1033010330
ProcessDeclAttributes(S, NewFD, D);
1033110331
const auto *NewTVA = NewFD->getAttr<TargetVersionAttr>();
10332-
if (NewTVA && !NewTVA->isDefaultVersion() &&
10332+
if (Context.getTargetInfo().getTriple().isAArch64() && NewTVA &&
10333+
!NewTVA->isDefaultVersion() &&
1033310334
!Context.getTargetInfo().hasFeature("fmv")) {
1033410335
// Don't add to scope fmv functions declarations if fmv disabled
1033510336
AddToScope = false;
@@ -11038,7 +11039,16 @@ static bool CheckMultiVersionValue(Sema &S, const FunctionDecl *FD) {
1103811039

1103911040
if (TVA) {
1104011041
llvm::SmallVector<StringRef, 8> Feats;
11041-
TVA->getFeatures(Feats);
11042+
ParsedTargetAttr ParseInfo;
11043+
if (S.getASTContext().getTargetInfo().getTriple().isRISCV()) {
11044+
ParseInfo =
11045+
S.getASTContext().getTargetInfo().parseTargetAttr(TVA->getName());
11046+
for (auto &Feat : ParseInfo.Features)
11047+
Feats.push_back(StringRef{Feat}.substr(1));
11048+
} else {
11049+
assert(S.getASTContext().getTargetInfo().getTriple().isAArch64());
11050+
TVA->getFeatures(Feats);
11051+
}
1104211052
for (const auto &Feat : Feats) {
1104311053
if (!TargetInfo.validateCpuSupports(Feat)) {
1104411054
S.Diag(FD->getLocation(), diag::err_bad_multiversion_option)
@@ -11324,7 +11334,8 @@ static bool PreviousDeclsHaveMultiVersionAttribute(const FunctionDecl *FD) {
1132411334
}
1132511335

1132611336
static void patchDefaultTargetVersion(FunctionDecl *From, FunctionDecl *To) {
11327-
if (!From->getASTContext().getTargetInfo().getTriple().isAArch64())
11337+
if (!From->getASTContext().getTargetInfo().getTriple().isAArch64() &&
11338+
!From->getASTContext().getTargetInfo().getTriple().isRISCV())
1132811339
return;
1132911340

1133011341
MultiVersionKind MVKindFrom = From->getMultiVersionKind();
@@ -15511,7 +15522,8 @@ Decl *Sema::ActOnStartOfFunctionDef(Scope *FnBodyScope, Decl *D,
1551115522
FD->setInvalidDecl();
1551215523
}
1551315524
if (const auto *Attr = FD->getAttr<TargetVersionAttr>()) {
15514-
if (!Context.getTargetInfo().hasFeature("fmv") &&
15525+
if (Context.getTargetInfo().getTriple().isAArch64() &&
15526+
!Context.getTargetInfo().hasFeature("fmv") &&
1551515527
!Attr->isDefaultVersion()) {
1551615528
// If function multi versioning disabled skip parsing function body
1551715529
// defined with non-default target_version attribute

clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3040,6 +3040,54 @@ bool Sema::checkTargetVersionAttr(SourceLocation LiteralLoc, Decl *D,
30403040
enum SecondParam { None };
30413041
enum ThirdParam { Target, TargetClones, TargetVersion };
30423042
llvm::SmallVector<StringRef, 8> Features;
3043+
if (Context.getTargetInfo().getTriple().isRISCV()) {
3044+
llvm::SmallVector<StringRef, 8> AttrStrs;
3045+
AttrStr.split(AttrStrs, ';');
3046+
3047+
bool HasArch = false;
3048+
bool HasPriority = false;
3049+
bool HasDefault = false;
3050+
bool DuplicateAttr = false;
3051+
for (auto &AttrStr : AttrStrs) {
3052+
// Only support arch=+ext,... syntax.
3053+
if (AttrStr.starts_with("arch=+")) {
3054+
if (HasArch)
3055+
DuplicateAttr = true;
3056+
HasArch = true;
3057+
ParsedTargetAttr TargetAttr =
3058+
Context.getTargetInfo().parseTargetAttr(AttrStr);
3059+
3060+
if (TargetAttr.Features.empty() ||
3061+
llvm::any_of(TargetAttr.Features, [&](const StringRef Ext) {
3062+
return !RISCV().isValidFMVExtension(Ext);
3063+
}))
3064+
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
3065+
<< Unsupported << None << AttrStr << TargetVersion;
3066+
} else if (AttrStr.starts_with("default")) {
3067+
if (HasDefault)
3068+
DuplicateAttr = true;
3069+
HasDefault = true;
3070+
} else if (AttrStr.consume_front("priority=")) {
3071+
if (HasPriority)
3072+
DuplicateAttr = true;
3073+
HasPriority = true;
3074+
int Digit;
3075+
if (AttrStr.getAsInteger(0, Digit))
3076+
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
3077+
<< Unsupported << None << AttrStr << TargetVersion;
3078+
} else {
3079+
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
3080+
<< Unsupported << None << AttrStr << TargetVersion;
3081+
}
3082+
}
3083+
3084+
if (((HasPriority || HasArch) && HasDefault) || DuplicateAttr ||
3085+
(HasPriority && !HasArch))
3086+
return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
3087+
<< Unsupported << None << AttrStr << TargetVersion;
3088+
3089+
return false;
3090+
}
30433091
AttrStr.split(Features, "+");
30443092
for (auto &CurFeature : Features) {
30453093
CurFeature = CurFeature.trim();
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// RUN: not %clang_cc1 -triple riscv64 -target-feature +i -emit-llvm -o - %s 2>&1 | FileCheck %s --check-prefix=CHECK-UNSUPPORT-OS
2+
3+
// CHECK-UNSUPPORT-OS: error: function multiversioning is currently only supported on Linux
4+
__attribute__((target_version("default"))) int foo(void) {
5+
return 2;
6+
}
7+
8+
__attribute__((target_version("arch=+c"))) int foo(void) {
9+
return 2;
10+
}
11+
12+
13+
int bar() { return foo(); }

0 commit comments

Comments
 (0)