[llvm] 124b46a - [NFC][AArch64] Document and improve FMV code.

Pavel Iliin via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 8 15:24:48 PST 2023


Author: Pavel Iliin
Date: 2023-03-08T23:22:46Z
New Revision: 124b46a897a7044be3cb49b9e0097d8cca47ba6b

URL: https://github.com/llvm/llvm-project/commit/124b46a897a7044be3cb49b9e0097d8cca47ba6b
DIFF: https://github.com/llvm/llvm-project/commit/124b46a897a7044be3cb49b9e0097d8cca47ba6b.diff

LOG: [NFC][AArch64] Document and improve FMV code.

Differential Revision: https://reviews.llvm.org/D145538

Added: 
    

Modified: 
    clang/include/clang/Basic/TargetInfo.h
    clang/lib/AST/ASTContext.cpp
    clang/lib/Basic/Targets/AArch64.cpp
    clang/lib/Basic/Targets/AArch64.h
    clang/lib/Sema/SemaDeclAttr.cpp
    llvm/include/llvm/TargetParser/AArch64TargetParser.h

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
index 1bdee3f416a90..6de5d90b12634 100644
--- a/clang/include/clang/Basic/TargetInfo.h
+++ b/clang/include/clang/Basic/TargetInfo.h
@@ -1333,12 +1333,16 @@ class TargetInfo : public virtual TransferrableTargetInfo,
   }
 
   /// Returns true if feature has an impact on target code
-  /// generation and get its dependent options in second argument.
-  virtual bool getFeatureDepOptions(StringRef Feature,
-                                    std::string &Options) const {
+  /// generation.
+  virtual bool doesFeatureAffectCodeGen(StringRef Feature) const {
     return true;
   }
 
+  /// For given feature return dependent ones.
+  virtual StringRef getFeatureDependencies(StringRef Feature) const {
+    return StringRef();
+  }
+
   struct BranchProtectionInfo {
     LangOptions::SignReturnAddressScopeKind SignReturnAddr =
         LangOptions::SignReturnAddressScopeKind::None;

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 1995030eec25f..90448c378458b 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -13456,6 +13456,7 @@ std::vector<std::string> ASTContext::filterFunctionTargetVersionAttrs(
   TV->getFeatures(Feats);
   for (auto &Feature : Feats)
     if (Target->validateCpuSupports(Feature.str()))
+      // Use '?' to mark features that came from TargetVersion.
       ResFeats.push_back("?" + Feature.str());
   return ResFeats;
 }
@@ -13525,6 +13526,7 @@ void ASTContext::getFunctionFeatureMap(llvm::StringMap<bool> &FeatureMap,
         VersionStr.split(VersionFeatures, "+");
         for (auto &VFeature : VersionFeatures) {
           VFeature = VFeature.trim();
+          // Use '?' to mark features that came from AArch64 TargetClones.
           Features.push_back((StringRef{"?"} + VFeature).str());
         }
       }

diff  --git a/clang/lib/Basic/Targets/AArch64.cpp b/clang/lib/Basic/Targets/AArch64.cpp
index 9b67becff9e75..7f331004348f1 100644
--- a/clang/lib/Basic/Targets/AArch64.cpp
+++ b/clang/lib/Basic/Targets/AArch64.cpp
@@ -606,16 +606,18 @@ unsigned AArch64TargetInfo::multiVersionFeatureCost() const {
   return llvm::AArch64::ExtensionInfo::MaxFMVPriority;
 }
 
-bool AArch64TargetInfo::getFeatureDepOptions(StringRef Name,
-                                             std::string &FeatureVec) const {
-  FeatureVec = "";
-  for (const auto &E : llvm::AArch64::Extensions) {
-    if (Name == E.Name) {
-      FeatureVec = E.DependentFeatures;
-      break;
-    }
-  }
-  return FeatureVec != "";
+bool AArch64TargetInfo::doesFeatureAffectCodeGen(StringRef Name) const {
+  auto F = llvm::find_if(llvm::AArch64::Extensions, [&](const auto &E) {
+    return Name == E.Name && !E.DependentFeatures.empty();
+  });
+  return F != std::end(llvm::AArch64::Extensions);
+}
+
+StringRef AArch64TargetInfo::getFeatureDependencies(StringRef Name) const {
+  auto F = llvm::find_if(llvm::AArch64::Extensions,
+                         [&](const auto &E) { return Name == E.Name; });
+  return F != std::end(llvm::AArch64::Extensions) ? F->DependentFeatures
+                                                  : StringRef();
 }
 
 bool AArch64TargetInfo::validateCpuSupports(StringRef FeatureStr) const {
@@ -962,18 +964,18 @@ bool AArch64TargetInfo::initFeatureMap(
   }
 
   // Process target and dependent features. This is done in two loops collecting
-  // them into UpdatedFeaturesVec: first to add dependent '+'features,
-  // second to add target '+/-'features that can later disable some of
-  // features added on the first loop.
+  // them into UpdatedFeaturesVec: first to add dependent '+'features, second to
+  // add target '+/-'features that can later disable some of features added on
+  // the first loop. Function Multi Versioning features begin with '?'.
   for (const auto &Feature : FeaturesVec)
-    if ((Feature[0] == '?' || Feature[0] == '+')) {
-      std::string Options;
-      if (AArch64TargetInfo::getFeatureDepOptions(Feature.substr(1), Options)) {
-        SmallVector<StringRef, 1> AttrFeatures;
-        StringRef(Options).split(AttrFeatures, ",");
-        for (auto F : AttrFeatures)
-          UpdatedFeaturesVec.push_back(F.str());
-      }
+    if (((Feature[0] == '?' || Feature[0] == '+')) &&
+        AArch64TargetInfo::doesFeatureAffectCodeGen(Feature.substr(1))) {
+      StringRef DepFeatures =
+          AArch64TargetInfo::getFeatureDependencies(Feature.substr(1));
+      SmallVector<StringRef, 1> AttrFeatures;
+      DepFeatures.split(AttrFeatures, ",");
+      for (auto F : AttrFeatures)
+        UpdatedFeaturesVec.push_back(F.str());
     }
   for (const auto &Feature : FeaturesVec)
     if (Feature[0] != '?') {

diff  --git a/clang/lib/Basic/Targets/AArch64.h b/clang/lib/Basic/Targets/AArch64.h
index 46bfb70a0d953..ee2c179d7c3df 100644
--- a/clang/lib/Basic/Targets/AArch64.h
+++ b/clang/lib/Basic/Targets/AArch64.h
@@ -143,9 +143,8 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo {
 
   std::optional<std::pair<unsigned, unsigned>>
   getVScaleRange(const LangOptions &LangOpts) const override;
-
-  bool getFeatureDepOptions(StringRef Feature,
-                            std::string &Options) const override;
+  bool doesFeatureAffectCodeGen(StringRef Name) const override;
+  StringRef getFeatureDependencies(StringRef Name) const override;
   bool validateCpuSupports(StringRef FeatureStr) const override;
   bool hasFeature(StringRef Feature) const override;
   void setFeatureEnabled(llvm::StringMap<bool> &Features, StringRef Name,

diff  --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 5a6e07c81041b..229e73618c53c 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3508,6 +3508,7 @@ bool Sema::checkTargetClonesAttrString(
   enum SecondParam { None, CPU, Tune };
   enum ThirdParam { Target, TargetClones };
   HasCommas = HasCommas || Str.contains(',');
+  const TargetInfo &TInfo = Context.getTargetInfo();
   // Warn on empty at the beginning of a string.
   if (Str.size() == 0)
     return Diag(LiteralLoc, diag::warn_unsupported_target_attribute)
@@ -3517,9 +3518,9 @@ bool Sema::checkTargetClonesAttrString(
   while (!Parts.second.empty()) {
     Parts = Parts.second.split(',');
     StringRef Cur = Parts.first.trim();
-    SourceLocation CurLoc = Literal->getLocationOfByte(
-        Cur.data() - Literal->getString().data(), getSourceManager(),
-        getLangOpts(), Context.getTargetInfo());
+    SourceLocation CurLoc =
+        Literal->getLocationOfByte(Cur.data() - Literal->getString().data(),
+                                   getSourceManager(), getLangOpts(), TInfo);
 
     bool DefaultIsDupe = false;
     bool HasCodeGenImpact = false;
@@ -3527,7 +3528,7 @@ bool Sema::checkTargetClonesAttrString(
       return Diag(CurLoc, diag::warn_unsupported_target_attribute)
              << Unsupported << None << "" << TargetClones;
 
-    if (Context.getTargetInfo().getTriple().isAArch64()) {
+    if (TInfo.getTriple().isAArch64()) {
       // AArch64 target clones specific
       if (Cur == "default") {
         DefaultIsDupe = HasDefault;
@@ -3542,13 +3543,12 @@ bool Sema::checkTargetClonesAttrString(
         while (!CurParts.second.empty()) {
           CurParts = CurParts.second.split('+');
           StringRef CurFeature = CurParts.first.trim();
-          if (!Context.getTargetInfo().validateCpuSupports(CurFeature)) {
+          if (!TInfo.validateCpuSupports(CurFeature)) {
             Diag(CurLoc, diag::warn_unsupported_target_attribute)
                 << Unsupported << None << CurFeature << TargetClones;
             continue;
           }
-          std::string Options;
-          if (Context.getTargetInfo().getFeatureDepOptions(CurFeature, Options))
+          if (TInfo.doesFeatureAffectCodeGen(CurFeature))
             HasCodeGenImpact = true;
           CurFeatures.push_back(CurFeature);
         }

diff  --git a/llvm/include/llvm/TargetParser/AArch64TargetParser.h b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
index 3e800bdfc61f4..133d10c11e57b 100644
--- a/llvm/include/llvm/TargetParser/AArch64TargetParser.h
+++ b/llvm/include/llvm/TargetParser/AArch64TargetParser.h
@@ -25,6 +25,9 @@ namespace llvm {
 class Triple;
 
 namespace AArch64 {
+// Function Multi Versioning CPU features. They must be kept in sync with
+// compiler-rt enum CPUFeatures in lib/builtins/cpu_model.c with FEAT_MAX as
+// sentinel.
 enum CPUFeatures {
   FEAT_RNG,
   FEAT_FLAGM,
@@ -87,6 +90,9 @@ enum CPUFeatures {
   FEAT_MAX
 };
 
+static_assert(FEAT_MAX <= 64,
+              "CPUFeatures enum must not have more than 64 entries");
+
 // Arch extension modifiers for CPUs. These are labelled with their Arm ARM
 // feature name (though the canonical reference for those is AArch64.td)
 // clang-format off
@@ -155,17 +161,18 @@ enum ArchExtKind : uint64_t {
 // SubtargetFeature which may represent either an actual extension or some
 // internal LLVM property.
 struct ExtensionInfo {
-  StringRef Name;       // Human readable name, e.g. "profile".
-  ArchExtKind ID;       // Corresponding to the ArchExtKind, this extensions
-                        // representation in the bitfield.
-  StringRef Feature;    // -mattr enable string, e.g. "+spe"
-  StringRef NegFeature; // -mattr disable string, e.g. "-spe"
-
-  // FIXME These were added by D127812 FMV support and need documenting:
-  CPUFeatures CPUFeature; // Bitfield value set in __aarch64_cpu_features
-  StringRef DependentFeatures;
-  unsigned FmvPriority;
-  static constexpr unsigned MaxFMVPriority = 1000;
+  StringRef Name;              // Human readable name, e.g. "profile".
+  ArchExtKind ID;              // Corresponding to the ArchExtKind, this
+                               // extensions representation in the bitfield.
+  StringRef Feature;           // -mattr enable string, e.g. "+spe"
+  StringRef NegFeature;        // -mattr disable string, e.g. "-spe"
+  CPUFeatures CPUFeature;      // Function Multi Versioning (FMV) bitfield value
+                               // set in __aarch64_cpu_features
+  StringRef DependentFeatures; // FMV enabled features string,
+                               // e.g. "+dotprod,+fp-armv8,+neon"
+  unsigned FmvPriority;        // FMV feature priority
+  static constexpr unsigned MaxFMVPriority =
+      1000; // Maximum priority for FMV feature
 };
 
 // clang-format off
@@ -559,6 +566,10 @@ std::optional<CpuInfo> parseCpu(StringRef Name);
 void fillValidCPUArchList(SmallVectorImpl<StringRef> &Values);
 
 bool isX18ReservedByDefault(const Triple &TT);
+
+// For given feature names, return a bitmask corresponding to the entries of
+// AArch64::CPUFeatures. The values in CPUFeatures are not bitmasks
+// themselves, they are sequential (0, 1, 2, 3, ...).
 uint64_t getCpuSupportsMask(ArrayRef<StringRef> FeatureStrs);
 
 } // namespace AArch64


        


More information about the llvm-commits mailing list