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 <l...@igalia.com> Date: Tue, 9 Jan 2024 15:32:15 +0700 Subject: [PATCH 1/3] [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 <l...@igalia.com> Date: Mon, 15 Jan 2024 12:20:25 +0700 Subject: [PATCH 2/3] 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 <l...@igalia.com> Date: Wed, 17 Jan 2024 11:01:47 +0700 Subject: [PATCH 3/3] 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), _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
