[clang] [RISCV] Overwrite cpu target features for full arch string in target attribute (PR #77426)
Luke Lau via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 16 20:50:01 PST 2024
https://github.com/lukel97 updated https://github.com/llvm/llvm-project/pull/77426
>From 0fadce20076015fbb28d449a2b3086f2e4261604 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Tue, 9 Jan 2024 15:32:15 +0700
Subject: [PATCH 1/4] [RISCV] Overwrite cpu target features for full arch
string in target attribute
This patch reworks RISCVTargetInfo::initFeatureMap to fix the issue described
in https://github.com/llvm/llvm-project/pull/74889#pullrequestreview-1773445559
(and is an alternative to #75804)
When a full arch string is specified, a "full" list of extensions is now passed
after the __RISCV_TargetAttrNeedOverride marker feature, which includes any
negative features that disable ISA extensions.
In initFeatureMap, there are now two code paths:
1. If the arch string was overriden, use the "full" list of override features,
only adding back any non-isa features that were specified.
Using the full list of positive and negative features will mean that the
target-cpu will have no effect on the final arch, e.g.
__attribute__((target("arch=rv64i"))) with -mcpu=sifive-x280 will have the
features for rv64i, not a mix of both.
2. Otherwise, parse and *append* the list of implied features. By appending, we
turn back on any features that might have been disabled by a negative
extension, i.e. this handles the case fixed in #74889.
---
clang/lib/Basic/Targets/RISCV.cpp | 78 +++++++------------
.../CodeGen/RISCV/riscv-func-attr-target.c | 8 +-
2 files changed, 30 insertions(+), 56 deletions(-)
diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index daaa8639ae8358..b56c1d465ad77a 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -235,39 +235,6 @@ ArrayRef<Builtin::Info> RISCVTargetInfo::getTargetBuiltins() const {
clang::RISCV::LastTSBuiltin - Builtin::FirstTSBuiltin);
}
-static std::vector<std::string>
-collectNonISAExtFeature(ArrayRef<std::string> FeaturesNeedOverride, int XLen) {
- std::vector<std::string> NonISAExtFeatureVec;
-
- auto IsNonISAExtFeature = [](const std::string &Feature) {
- assert(Feature.size() > 1 && (Feature[0] == '+' || Feature[0] == '-'));
- StringRef Ext = StringRef(Feature).drop_front(); // drop the +/-
- return !llvm::RISCVISAInfo::isSupportedExtensionFeature(Ext);
- };
- llvm::copy_if(FeaturesNeedOverride, std::back_inserter(NonISAExtFeatureVec),
- IsNonISAExtFeature);
-
- return NonISAExtFeatureVec;
-}
-
-static std::vector<std::string>
-resolveTargetAttrOverride(const std::vector<std::string> &FeaturesVec,
- int XLen) {
- auto I = llvm::find(FeaturesVec, "__RISCV_TargetAttrNeedOverride");
- if (I == FeaturesVec.end())
- return FeaturesVec;
-
- ArrayRef<std::string> FeaturesNeedOverride(&*FeaturesVec.begin(), &*I);
- std::vector<std::string> NonISAExtFeature =
- collectNonISAExtFeature(FeaturesNeedOverride, XLen);
-
- std::vector<std::string> ResolvedFeature(++I, FeaturesVec.end());
- ResolvedFeature.insert(ResolvedFeature.end(), NonISAExtFeature.begin(),
- NonISAExtFeature.end());
-
- return ResolvedFeature;
-}
-
bool RISCVTargetInfo::initFeatureMap(
llvm::StringMap<bool> &Features, DiagnosticsEngine &Diags, StringRef CPU,
const std::vector<std::string> &FeaturesVec) const {
@@ -281,10 +248,27 @@ bool RISCVTargetInfo::initFeatureMap(
Features["32bit"] = true;
}
- std::vector<std::string> NewFeaturesVec =
- resolveTargetAttrOverride(FeaturesVec, XLen);
+ // If a target attribute specified a full arch string, override all the ISA
+ // extension target features.
+ const auto I = llvm::find(FeaturesVec, "__RISCV_TargetAttrNeedOverride");
+ if (I != FeaturesVec.end()) {
+ std::vector OverrideFeatures = std::vector(std::next(I), FeaturesVec.end());
+
+ // Add back any non ISA extension features, e.g. +relax.
+ auto IsNonISAExtFeature = [](const std::string &Feature) {
+ assert(Feature.size() > 1 && (Feature[0] == '+' || Feature[0] == '-'));
+ std::string Ext = Feature.substr(1); // drop the +/-
+ return !llvm::RISCVISAInfo::isSupportedExtensionFeature(Ext);
+ };
+ llvm::copy_if(llvm::make_range(FeaturesVec.begin(), I),
+ std::back_inserter(OverrideFeatures), IsNonISAExtFeature);
- auto ParseResult = llvm::RISCVISAInfo::parseFeatures(XLen, NewFeaturesVec);
+ return TargetInfo::initFeatureMap(Features, Diags, CPU, OverrideFeatures);
+ }
+
+ // Otherwise, parse the features and add any implied extensions.
+ std::vector AllFeatures = FeaturesVec;
+ auto ParseResult = llvm::RISCVISAInfo::parseFeatures(XLen, FeaturesVec);
if (!ParseResult) {
std::string Buffer;
llvm::raw_string_ostream OutputErrMsg(Buffer);
@@ -295,21 +279,9 @@ bool RISCVTargetInfo::initFeatureMap(
return false;
}
- // RISCVISAInfo makes implications for ISA features
- std::vector<std::string> ImpliedFeatures = (*ParseResult)->toFeatures();
-
- // parseFeatures normalizes the feature set by dropping any explicit
- // negatives, and non-extension features. We need to preserve the later
- // for correctness and want to preserve the former for consistency.
- for (auto &Feature : NewFeaturesVec) {
- StringRef ExtName = Feature;
- assert(ExtName.size() > 1 && (ExtName[0] == '+' || ExtName[0] == '-'));
- ExtName = ExtName.drop_front(1); // Drop '+' or '-'
- if (!llvm::is_contained(ImpliedFeatures, ("+" + ExtName).str()) &&
- !llvm::is_contained(ImpliedFeatures, ("-" + ExtName).str()))
- ImpliedFeatures.push_back(Feature);
- }
- return TargetInfo::initFeatureMap(Features, Diags, CPU, ImpliedFeatures);
+ // Append all features, not just new ones, so we override any negatives.
+ llvm::append_range(AllFeatures, (*ParseResult)->toFeatures());
+ return TargetInfo::initFeatureMap(Features, Diags, CPU, AllFeatures);
}
std::optional<std::pair<unsigned, unsigned>>
@@ -413,7 +385,9 @@ static void handleFullArchString(StringRef FullArchStr,
// Forward the invalid FullArchStr.
Features.push_back("+" + FullArchStr.str());
} else {
- std::vector<std::string> FeatStrings = (*RII)->toFeatures();
+ // Append a full list of features, including any negative extensions so that
+ // we override the CPU's features.
+ std::vector<std::string> FeatStrings = (*RII)->toFeatures(true);
Features.insert(Features.end(), FeatStrings.begin(), FeatStrings.end());
}
}
diff --git a/clang/test/CodeGen/RISCV/riscv-func-attr-target.c b/clang/test/CodeGen/RISCV/riscv-func-attr-target.c
index 759c33a2250600..54a0aad8a18b43 100644
--- a/clang/test/CodeGen/RISCV/riscv-func-attr-target.c
+++ b/clang/test/CodeGen/RISCV/riscv-func-attr-target.c
@@ -40,8 +40,8 @@ __attribute__((target("cpu=sifive-u54"))) void testAttrCpuOnly() {}
// CHECK: attributes #1 = { {{.*}}"target-cpu"="rocket-rv64" "target-features"="+64bit,+a,+d,+f,+m,+save-restore,+v,+zicsr,+zifencei,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b,-relax,-zbb,-zfa" "tune-cpu"="generic-rv64" }
// CHECK: attributes #2 = { {{.*}}"target-features"="+64bit,+a,+m,+save-restore,+zbb,+zifencei,-relax,-zfa" }
// CHECK: attributes #3 = { {{.*}}"target-features"="+64bit,+a,+d,+experimental-zicond,+f,+m,+save-restore,+v,+zbb,+zicsr,+zifencei,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b,-relax,-zfa" }
-// CHECK: attributes #4 = { {{.*}}"target-features"="+64bit,+a,+c,+d,+f,+m,+save-restore,+zbb,+zicsr,+zifencei,-relax" }
-// CHECK: attributes #5 = { {{.*}}"target-features"="+64bit,+m,+save-restore,-relax" }
+// CHECK: attributes #4 = { {{.*}}"target-features"="+64bit,+a,+c,+d,+f,+m,+save-restore,+zbb,+zicsr,+zifencei,-e,-experimental-zacas,-experimental-zcmop,-experimental-zfbfmin,-experimental-zicfilp,-experimental-zicfiss,-experimental-zicond,-experimental-zimop,-experimental-ztso,-experimental-zvfbfmin,-experimental-zvfbfwma,-h,-relax,-smaia,-ssaia,-svinval,-svnapot,-svpbmt,-v,-xcvalu,-xcvbi,-xcvbitmanip,-xcvelw,-xcvmac,-xcvmem,-xcvsimd,-xsfvcp,-xsfvfnrclipxfqf,-xsfvfwmaccqqq,-xsfvqmaccdod,-xsfvqmaccqoq,-xtheadba,-xtheadbb,-xtheadbs,-xtheadcmo,-xtheadcondmov,-xtheadfmemidx,-xtheadmac,-xtheadmemidx,-xtheadmempair,-xtheadsync,-xtheadvdot,-xventanacondops,-zawrs,-zba,-zbc,-zbkb,-zbkc,-zbkx,-zbs,-zca,-zcb,-zcd,-zce,-zcf,-zcmp,-zcmt,-zdinx,-zfa,-zfh,-zfhmin,-zfinx,-zhinx,-zhinxmin,-zicbom,-zicbop,-zicboz,-zicntr,-zihintntl,-zihintpause,-zihpm,-zk,-zkn,-zknd,-zkne,-zknh,-zkr,-zks,-zksed,-zksh,-zkt,-zmmul,-zvbb,-zvbc,-zve32f,-zve32x,-zve64d,-zve64f,-zve64x,-zvfh,-zvfhmin,-zvkb,-zvkg,-zvkn,-zvknc,-zvkned,-zvkng,-zvknha,-zvknhb,-zvks,-zvksc,-zvksed,-zvksg,-zvksh,-zvkt,-zvl1024b,-zvl128b,-zvl16384b,-zvl2048b,-zvl256b,-zvl32768b,-zvl32b,-zvl4096b,-zvl512b,-zvl64b,-zvl65536b,-zvl8192b" }
+// CHECK: attributes #5 = { {{.*}}"target-features"="+64bit,+m,+save-restore,-a,-c,-d,-e,-experimental-zacas,-experimental-zcmop,-experimental-zfbfmin,-experimental-zicfilp,-experimental-zicfiss,-experimental-zicond,-experimental-zimop,-experimental-ztso,-experimental-zvfbfmin,-experimental-zvfbfwma,-f,-h,-relax,-smaia,-ssaia,-svinval,-svnapot,-svpbmt,-v,-xcvalu,-xcvbi,-xcvbitmanip,-xcvelw,-xcvmac,-xcvmem,-xcvsimd,-xsfvcp,-xsfvfnrclipxfqf,-xsfvfwmaccqqq,-xsfvqmaccdod,-xsfvqmaccqoq,-xtheadba,-xtheadbb,-xtheadbs,-xtheadcmo,-xtheadcondmov,-xtheadfmemidx,-xtheadmac,-xtheadmemidx,-xtheadmempair,-xtheadsync,-xtheadvdot,-xventanacondops,-zawrs,-zba,-zbb,-zbc,-zbkb,-zbkc,-zbkx,-zbs,-zca,-zcb,-zcd,-zce,-zcf,-zcmp,-zcmt,-zdinx,-zfa,-zfh,-zfhmin,-zfinx,-zhinx,-zhinxmin,-zicbom,-zicbop,-zicboz,-zicntr,-zicsr,-zifencei,-zihintntl,-zihintpause,-zihpm,-zk,-zkn,-zknd,-zkne,-zknh,-zkr,-zks,-zksed,-zksh,-zkt,-zmmul,-zvbb,-zvbc,-zve32f,-zve32x,-zve64d,-zve64f,-zve64x,-zvfh,-zvfhmin,-zvkb,-zvkg,-zvkn,-zvknc,-zvkned,-zvkng,-zvknha,-zvknhb,-zvks,-zvksc,-zvksed,-zvksg,-zvksh,-zvkt,-zvl1024b,-zvl128b,-zvl16384b,-zvl2048b,-zvl256b,-zvl32768b,-zvl32b,-zvl4096b,-zvl512b,-zvl64b,-zvl65536b,-zvl8192b" }
// CHECK: attributes #6 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+a,+m,+save-restore,+zbb,+zifencei,-relax,-zfa" }
-// CHECK: attributes #7 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+m,+save-restore,-relax" }
-// CHECK: attributes #8 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+a,+c,+d,+f,+m,+save-restore,+zicsr,+zifencei,-relax" }
+// CHECK: attributes #7 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+m,+save-restore,-a,-c,-d,-e,-experimental-zacas,-experimental-zcmop,-experimental-zfbfmin,-experimental-zicfilp,-experimental-zicfiss,-experimental-zicond,-experimental-zimop,-experimental-ztso,-experimental-zvfbfmin,-experimental-zvfbfwma,-f,-h,-relax,-smaia,-ssaia,-svinval,-svnapot,-svpbmt,-v,-xcvalu,-xcvbi,-xcvbitmanip,-xcvelw,-xcvmac,-xcvmem,-xcvsimd,-xsfvcp,-xsfvfnrclipxfqf,-xsfvfwmaccqqq,-xsfvqmaccdod,-xsfvqmaccqoq,-xtheadba,-xtheadbb,-xtheadbs,-xtheadcmo,-xtheadcondmov,-xtheadfmemidx,-xtheadmac,-xtheadmemidx,-xtheadmempair,-xtheadsync,-xtheadvdot,-xventanacondops,-zawrs,-zba,-zbb,-zbc,-zbkb,-zbkc,-zbkx,-zbs,-zca,-zcb,-zcd,-zce,-zcf,-zcmp,-zcmt,-zdinx,-zfa,-zfh,-zfhmin,-zfinx,-zhinx,-zhinxmin,-zicbom,-zicbop,-zicboz,-zicntr,-zicsr,-zifencei,-zihintntl,-zihintpause,-zihpm,-zk,-zkn,-zknd,-zkne,-zknh,-zkr,-zks,-zksed,-zksh,-zkt,-zmmul,-zvbb,-zvbc,-zve32f,-zve32x,-zve64d,-zve64f,-zve64x,-zvfh,-zvfhmin,-zvkb,-zvkg,-zvkn,-zvknc,-zvkned,-zvkng,-zvknha,-zvknhb,-zvks,-zvksc,-zvksed,-zvksg,-zvksh,-zvkt,-zvl1024b,-zvl128b,-zvl16384b,-zvl2048b,-zvl256b,-zvl32768b,-zvl32b,-zvl4096b,-zvl512b,-zvl64b,-zvl65536b,-zvl8192b" }
+// CHECK: attributes #8 = { {{.*}}"target-cpu"="sifive-u54" "target-features"="+64bit,+a,+c,+d,+f,+m,+save-restore,+zicsr,+zifencei,-e,-experimental-zacas,-experimental-zcmop,-experimental-zfbfmin,-experimental-zicfilp,-experimental-zicfiss,-experimental-zicond,-experimental-zimop,-experimental-ztso,-experimental-zvfbfmin,-experimental-zvfbfwma,-h,-relax,-smaia,-ssaia,-svinval,-svnapot,-svpbmt,-v,-xcvalu,-xcvbi,-xcvbitmanip,-xcvelw,-xcvmac,-xcvmem,-xcvsimd,-xsfvcp,-xsfvfnrclipxfqf,-xsfvfwmaccqqq,-xsfvqmaccdod,-xsfvqmaccqoq,-xtheadba,-xtheadbb,-xtheadbs,-xtheadcmo,-xtheadcondmov,-xtheadfmemidx,-xtheadmac,-xtheadmemidx,-xtheadmempair,-xtheadsync,-xtheadvdot,-xventanacondops,-zawrs,-zba,-zbb,-zbc,-zbkb,-zbkc,-zbkx,-zbs,-zca,-zcb,-zcd,-zce,-zcf,-zcmp,-zcmt,-zdinx,-zfa,-zfh,-zfhmin,-zfinx,-zhinx,-zhinxmin,-zicbom,-zicbop,-zicboz,-zicntr,-zihintntl,-zihintpause,-zihpm,-zk,-zkn,-zknd,-zkne,-zknh,-zkr,-zks,-zksed,-zksh,-zkt,-zmmul,-zvbb,-zvbc,-zve32f,-zve32x,-zve64d,-zve64f,-zve64x,-zvfh,-zvfhmin,-zvkb,-zvkg,-zvkn,-zvknc,-zvkned,-zvkng,-zvknha,-zvknhb,-zvks,-zvksc,-zvksed,-zvksg,-zvksh,-zvkt,-zvl1024b,-zvl128b,-zvl16384b,-zvl2048b,-zvl256b,-zvl32768b,-zvl32b,-zvl4096b,-zvl512b,-zvl64b,-zvl65536b,-zvl8192b" }
>From 82536a4d13e085803da255ebcf654ac8281e5f7e Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Mon, 15 Jan 2024 12:20:25 +0700
Subject: [PATCH 2/4] Use std::vector<std::string>, add comment for
AddAllExtensions argument
---
clang/lib/Basic/Targets/RISCV.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index b56c1d465ad77a..c46be2300bcb54 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -252,7 +252,8 @@ bool RISCVTargetInfo::initFeatureMap(
// extension target features.
const auto I = llvm::find(FeaturesVec, "__RISCV_TargetAttrNeedOverride");
if (I != FeaturesVec.end()) {
- std::vector OverrideFeatures = std::vector(std::next(I), FeaturesVec.end());
+ std::vector<std::string> OverrideFeatures =
+ std::vector(std::next(I), FeaturesVec.end());
// Add back any non ISA extension features, e.g. +relax.
auto IsNonISAExtFeature = [](const std::string &Feature) {
@@ -387,7 +388,8 @@ static void handleFullArchString(StringRef FullArchStr,
} else {
// Append a full list of features, including any negative extensions so that
// we override the CPU's features.
- std::vector<std::string> FeatStrings = (*RII)->toFeatures(true);
+ std::vector<std::string> FeatStrings =
+ (*RII)->toFeatures(/* AddAllExtensions */ true);
Features.insert(Features.end(), FeatStrings.begin(), FeatStrings.end());
}
}
>From 33fe75c57cefee260c3a072cd768dcf8250d83f7 Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Wed, 17 Jan 2024 11:01:47 +0700
Subject: [PATCH 3/4] Use StringRef, use direct initialization for std::vector
---
clang/lib/Basic/Targets/RISCV.cpp | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index c46be2300bcb54..90ffd421c25d17 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -252,13 +252,12 @@ bool RISCVTargetInfo::initFeatureMap(
// extension target features.
const auto I = llvm::find(FeaturesVec, "__RISCV_TargetAttrNeedOverride");
if (I != FeaturesVec.end()) {
- std::vector<std::string> OverrideFeatures =
- std::vector(std::next(I), FeaturesVec.end());
+ std::vector<std::string> OverrideFeatures(std::next(I), FeaturesVec.end());
// Add back any non ISA extension features, e.g. +relax.
- auto IsNonISAExtFeature = [](const std::string &Feature) {
+ auto IsNonISAExtFeature = [](StringRef Feature) {
assert(Feature.size() > 1 && (Feature[0] == '+' || Feature[0] == '-'));
- std::string Ext = Feature.substr(1); // drop the +/-
+ StringRef Ext = Feature.substr(1); // drop the +/-
return !llvm::RISCVISAInfo::isSupportedExtensionFeature(Ext);
};
llvm::copy_if(llvm::make_range(FeaturesVec.begin(), I),
>From 9f270a63e591d28a913ed895758b95e1c96c23eb Mon Sep 17 00:00:00 2001
From: Luke Lau <luke at igalia.com>
Date: Wed, 17 Jan 2024 11:49:10 +0700
Subject: [PATCH 4/4] Add more explicit template arguments to std::vector
---
clang/lib/Basic/Targets/RISCV.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Basic/Targets/RISCV.cpp b/clang/lib/Basic/Targets/RISCV.cpp
index 90ffd421c25d17..6bd308728f082e 100644
--- a/clang/lib/Basic/Targets/RISCV.cpp
+++ b/clang/lib/Basic/Targets/RISCV.cpp
@@ -267,7 +267,7 @@ bool RISCVTargetInfo::initFeatureMap(
}
// Otherwise, parse the features and add any implied extensions.
- std::vector AllFeatures = FeaturesVec;
+ std::vector<std::string> AllFeatures = FeaturesVec;
auto ParseResult = llvm::RISCVISAInfo::parseFeatures(XLen, FeaturesVec);
if (!ParseResult) {
std::string Buffer;
More information about the cfe-commits
mailing list