[Subtarget] Move SubtargetFeatureKV/SubtargetInfoKV from SubtargetFeature.h to MCSubt...
authorCraig Topper <craig.topper@intel.com>
Tue, 5 Mar 2019 18:54:30 +0000 (18:54 +0000)
committerCraig Topper <craig.topper@intel.com>
Tue, 5 Mar 2019 18:54:30 +0000 (18:54 +0000)
The SubtargetFeature class managed a list of features as strings. And it also had functions for setting bits in a FeatureBitset.

The methods that operated on the Feature list as strings are used in other parts of the backend. But the parts that operate on FeatureBitset are very tightly coupled to MCSubtargetInfo and requires passing in the arrays that MCSubtargetInfo owns. And the same struct type is used for ProcFeatures and ProcDesc.

This has led to MCSubtargetInfo having 2 arrays keyed by CPU name. One containing a mapping from a CPU name to its features. And one containing a mapping from CPU name to its scheduler model.

I would like to make a single CPU array containing all CPU information and remove some unneeded fields the ProcDesc array currently has. But I don't want to make SubtargetFeatures.h have to know about the scheduler model type and have to forward declare or pull in the header file.

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

llvm-svn: 355428

llvm/include/llvm/MC/MCSubtargetInfo.h
llvm/include/llvm/MC/SubtargetFeature.h
llvm/lib/MC/MCSubtargetInfo.cpp
llvm/lib/MC/SubtargetFeature.cpp

index c7472a2..4da1664 100644 (file)
@@ -29,6 +29,44 @@ namespace llvm {
 class MCInst;
 
 //===----------------------------------------------------------------------===//
+
+/// Used to provide key value pairs for feature and CPU bit flags.
+struct SubtargetFeatureKV {
+  const char *Key;                      ///< K-V key string
+  const char *Desc;                     ///< Help descriptor
+  unsigned Value;                       ///< K-V integer value
+  FeatureBitArray Implies;              ///< K-V bit mask
+
+  /// Compare routine for std::lower_bound
+  bool operator<(StringRef S) const {
+    return StringRef(Key) < S;
+  }
+
+  /// Compare routine for std::is_sorted.
+  bool operator<(const SubtargetFeatureKV &Other) const {
+    return StringRef(Key) < StringRef(Other.Key);
+  }
+};
+
+//===----------------------------------------------------------------------===//
+
+/// Used to provide key value pairs for CPU and arbitrary pointers.
+struct SubtargetInfoKV {
+  const char *Key;                      ///< K-V key string
+  const void *Value;                    ///< K-V pointer value
+
+  /// Compare routine for std::lower_bound
+  bool operator<(StringRef S) const {
+    return StringRef(Key) < S;
+  }
+
+  /// Compare routine for std::is_sorted.
+  bool operator<(const SubtargetInfoKV &Other) const {
+    return StringRef(Key) < StringRef(Other.Key);
+  }
+};
+
+//===----------------------------------------------------------------------===//
 ///
 /// Generic base class for all target subtargets.
 ///
index ee27827..01e5677 100644 (file)
@@ -26,7 +26,6 @@
 
 namespace llvm {
 
-template <typename T> class ArrayRef;
 class raw_ostream;
 class Triple;
 
@@ -71,44 +70,6 @@ public:
 
 //===----------------------------------------------------------------------===//
 
-/// Used to provide key value pairs for feature and CPU bit flags.
-struct SubtargetFeatureKV {
-  const char *Key;                      ///< K-V key string
-  const char *Desc;                     ///< Help descriptor
-  unsigned Value;                       ///< K-V integer value
-  FeatureBitArray Implies;              ///< K-V bit mask
-
-  /// Compare routine for std::lower_bound
-  bool operator<(StringRef S) const {
-    return StringRef(Key) < S;
-  }
-
-  /// Compare routine for std::is_sorted.
-  bool operator<(const SubtargetFeatureKV &Other) const {
-    return StringRef(Key) < StringRef(Other.Key);
-  }
-};
-
-//===----------------------------------------------------------------------===//
-
-/// Used to provide key value pairs for CPU and arbitrary pointers.
-struct SubtargetInfoKV {
-  const char *Key;                      ///< K-V key string
-  const void *Value;                    ///< K-V pointer value
-
-  /// Compare routine for std::lower_bound
-  bool operator<(StringRef S) const {
-    return StringRef(Key) < S;
-  }
-
-  /// Compare routine for std::is_sorted.
-  bool operator<(const SubtargetInfoKV &Other) const {
-    return StringRef(Key) < StringRef(Other.Key);
-  }
-};
-
-//===----------------------------------------------------------------------===//
-
 /// Manages the enabling and disabling of subtarget specific features.
 ///
 /// Features are encoded as a string of the form
@@ -129,19 +90,6 @@ public:
   /// Adds Features.
   void AddFeature(StringRef String, bool Enable = true);
 
-  /// Toggles a feature and update the feature bits.
-  static void ToggleFeature(FeatureBitset &Bits, StringRef String,
-                            ArrayRef<SubtargetFeatureKV> FeatureTable);
-
-  /// Applies the feature flag and update the feature bits.
-  static void ApplyFeatureFlag(FeatureBitset &Bits, StringRef Feature,
-                               ArrayRef<SubtargetFeatureKV> FeatureTable);
-
-  /// Returns feature bits of a CPU.
-  FeatureBitset getFeatureBits(StringRef CPU,
-                               ArrayRef<SubtargetFeatureKV> CPUTable,
-                               ArrayRef<SubtargetFeatureKV> FeatureTable);
-
   /// Returns the vector of individual subtarget features.
   const std::vector<std::string> &getFeatures() const { return Features; }
 
@@ -153,6 +101,32 @@ public:
 
   /// Adds the default features for the specified target triple.
   void getDefaultSubtargetFeatures(const Triple& Triple);
+
+  /// Determine if a feature has a flag; '+' or '-'
+  static bool hasFlag(StringRef Feature) {
+    assert(!Feature.empty() && "Empty string");
+    // Get first character
+    char Ch = Feature[0];
+    // Check if first character is '+' or '-' flag
+    return Ch == '+' || Ch =='-';
+  }
+
+  /// Return string stripped of flag.
+  static std::string StripFlag(StringRef Feature) {
+    return hasFlag(Feature) ? Feature.substr(1) : Feature;
+  }
+
+  /// Return true if enable flag; '+'.
+  static inline bool isEnabled(StringRef Feature) {
+    assert(!Feature.empty() && "Empty string");
+    // Get first character
+    char Ch = Feature[0];
+    // Check if first character is '+' for enabled
+    return Ch == '+';
+  }
+
+  /// Splits a string of comma separated items in to a vector of strings.
+  static void Split(std::vector<std::string> &V, StringRef S);
 };
 
 } // end namespace llvm
index 9e7ea59..5cb51ed 100644 (file)
@@ -12,6 +12,7 @@
 #include "llvm/MC/MCInstrItineraries.h"
 #include "llvm/MC/MCSchedule.h"
 #include "llvm/MC/SubtargetFeature.h"
+#include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 
 using namespace llvm;
 
+/// Find KV in array using binary search.
+static const SubtargetFeatureKV *Find(StringRef S,
+                                      ArrayRef<SubtargetFeatureKV> A) {
+  // Binary search the array
+  auto F = std::lower_bound(A.begin(), A.end(), S);
+  // If not found then return NULL
+  if (F == A.end() || StringRef(F->Key) != S) return nullptr;
+  // Return the found array item
+  return F;
+}
+
+/// For each feature that is (transitively) implied by this feature, set it.
+static
+void SetImpliedBits(FeatureBitset &Bits, const FeatureBitset &Implies,
+                    ArrayRef<SubtargetFeatureKV> FeatureTable) {
+  // OR the Implies bits in outside the loop. This allows the Implies for CPUs
+  // which might imply features not in FeatureTable to use this.
+  Bits |= Implies;
+  for (const SubtargetFeatureKV &FE : FeatureTable)
+    if (Implies.test(FE.Value))
+      SetImpliedBits(Bits, FE.Implies.getAsBitset(), FeatureTable);
+}
+
+/// For each feature that (transitively) implies this feature, clear it.
+static
+void ClearImpliedBits(FeatureBitset &Bits, unsigned Value,
+                      ArrayRef<SubtargetFeatureKV> FeatureTable) {
+  for (const SubtargetFeatureKV &FE : FeatureTable) {
+    if (FE.Implies.getAsBitset().test(Value)) {
+      Bits.reset(FE.Value);
+      ClearImpliedBits(Bits, FE.Value, FeatureTable);
+    }
+  }
+}
+
+static void ApplyFeatureFlag(FeatureBitset &Bits, StringRef Feature,
+                             ArrayRef<SubtargetFeatureKV> FeatureTable) {
+  assert(SubtargetFeatures::hasFlag(Feature) &&
+         "Feature flags should start with '+' or '-'");
+
+  // Find feature in table.
+  const SubtargetFeatureKV *FeatureEntry =
+      Find(SubtargetFeatures::StripFlag(Feature), FeatureTable);
+  // If there is a match
+  if (FeatureEntry) {
+    // Enable/disable feature in bits
+    if (SubtargetFeatures::isEnabled(Feature)) {
+      Bits.set(FeatureEntry->Value);
+
+      // For each feature that this implies, set it.
+      SetImpliedBits(Bits, FeatureEntry->Implies.getAsBitset(), FeatureTable);
+    } else {
+      Bits.reset(FeatureEntry->Value);
+
+      // For each feature that implies this, clear it.
+      ClearImpliedBits(Bits, FeatureEntry->Value, FeatureTable);
+    }
+  } else {
+    errs() << "'" << Feature << "' is not a recognized feature for this target"
+           << " (ignoring feature)\n";
+  }
+}
+
+/// Return the length of the longest entry in the table.
+static size_t getLongestEntryLength(ArrayRef<SubtargetFeatureKV> Table) {
+  size_t MaxLen = 0;
+  for (auto &I : Table)
+    MaxLen = std::max(MaxLen, std::strlen(I.Key));
+  return MaxLen;
+}
+
+/// Display help for feature choices.
+static void Help(ArrayRef<SubtargetFeatureKV> CPUTable,
+                 ArrayRef<SubtargetFeatureKV> FeatTable) {
+  // Determine the length of the longest CPU and Feature entries.
+  unsigned MaxCPULen  = getLongestEntryLength(CPUTable);
+  unsigned MaxFeatLen = getLongestEntryLength(FeatTable);
+
+  // Print the CPU table.
+  errs() << "Available CPUs for this target:\n\n";
+  for (auto &CPU : CPUTable)
+    errs() << format("  %-*s - %s.\n", MaxCPULen, CPU.Key, CPU.Desc);
+  errs() << '\n';
+
+  // Print the Feature table.
+  errs() << "Available features for this target:\n\n";
+  for (auto &Feature : FeatTable)
+    errs() << format("  %-*s - %s.\n", MaxFeatLen, Feature.Key, Feature.Desc);
+  errs() << '\n';
+
+  errs() << "Use +feature to enable a feature, or -feature to disable it.\n"
+            "For example, llc -mcpu=mycpu -mattr=+feature1,-feature2\n";
+}
+
 static FeatureBitset getFeatures(StringRef CPU, StringRef FS,
                                  ArrayRef<SubtargetFeatureKV> ProcDesc,
                                  ArrayRef<SubtargetFeatureKV> ProcFeatures) {
   SubtargetFeatures Features(FS);
-  return Features.getFeatureBits(CPU, ProcDesc, ProcFeatures);
+
+  if (ProcDesc.empty() || ProcFeatures.empty())
+    return FeatureBitset();
+
+  assert(std::is_sorted(std::begin(ProcDesc), std::end(ProcDesc)) &&
+         "CPU table is not sorted");
+  assert(std::is_sorted(std::begin(ProcFeatures), std::end(ProcFeatures)) &&
+         "CPU features table is not sorted");
+  // Resulting bits
+  FeatureBitset Bits;
+
+  // Check if help is needed
+  if (CPU == "help")
+    Help(ProcDesc, ProcFeatures);
+
+  // Find CPU entry if CPU name is specified.
+  else if (!CPU.empty()) {
+    const SubtargetFeatureKV *CPUEntry = Find(CPU, ProcDesc);
+
+    // If there is a match
+    if (CPUEntry) {
+      // Set the features implied by this CPU feature, if any.
+      SetImpliedBits(Bits, CPUEntry->Implies.getAsBitset(), ProcFeatures);
+    } else {
+      errs() << "'" << CPU << "' is not a recognized processor for this target"
+             << " (ignoring processor)\n";
+    }
+  }
+
+  // Iterate through each feature
+  for (const std::string &Feature : Features.getFeatures()) {
+    // Check for help
+    if (Feature == "+help")
+      Help(ProcDesc, ProcFeatures);
+    else
+      ApplyFeatureFlag(Bits, Feature, ProcFeatures);
+  }
+
+  return Bits;
 }
 
 void MCSubtargetInfo::InitMCProcessorInfo(StringRef CPU, StringRef FS) {
@@ -60,13 +193,33 @@ FeatureBitset MCSubtargetInfo::ToggleFeature(const FeatureBitset &FB) {
   return FeatureBits;
 }
 
-FeatureBitset MCSubtargetInfo::ToggleFeature(StringRef FS) {
-  SubtargetFeatures::ToggleFeature(FeatureBits, FS, ProcFeatures);
+FeatureBitset MCSubtargetInfo::ToggleFeature(StringRef Feature) {
+  // Find feature in table.
+  const SubtargetFeatureKV *FeatureEntry =
+      Find(SubtargetFeatures::StripFlag(Feature), ProcFeatures);
+  // If there is a match
+  if (FeatureEntry) {
+    if (FeatureBits.test(FeatureEntry->Value)) {
+      FeatureBits.reset(FeatureEntry->Value);
+      // For each feature that implies this, clear it.
+      ClearImpliedBits(FeatureBits, FeatureEntry->Value, ProcFeatures);
+    } else {
+      FeatureBits.set(FeatureEntry->Value);
+
+      // For each feature that this implies, set it.
+      SetImpliedBits(FeatureBits, FeatureEntry->Implies.getAsBitset(),
+                     ProcFeatures);
+    }
+  } else {
+    errs() << "'" << Feature << "' is not a recognized feature for this target"
+           << " (ignoring feature)\n";
+  }
+
   return FeatureBits;
 }
 
 FeatureBitset MCSubtargetInfo::ApplyFeatureFlag(StringRef FS) {
-  SubtargetFeatures::ApplyFeatureFlag(FeatureBits, FS, ProcFeatures);
+  ::ApplyFeatureFlag(FeatureBits, FS, ProcFeatures);
   return FeatureBits;
 }
 
@@ -74,10 +227,10 @@ bool MCSubtargetInfo::checkFeatures(StringRef FS) const {
   SubtargetFeatures T(FS);
   FeatureBitset Set, All;
   for (std::string F : T.getFeatures()) {
-    SubtargetFeatures::ApplyFeatureFlag(Set, F, ProcFeatures);
+    ::ApplyFeatureFlag(Set, F, ProcFeatures);
     if (F[0] == '-')
       F[0] = '+';
-    SubtargetFeatures::ApplyFeatureFlag(All, F, ProcFeatures);
+    ::ApplyFeatureFlag(All, F, ProcFeatures);
   }
   return (FeatureBits & All) == Set;
 }
index 9b774c1..c4dd773 100644 (file)
@@ -11,7 +11,6 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/MC/SubtargetFeature.h"
-#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
@@ -19,7 +18,6 @@
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Debug.h"
-#include "llvm/Support/Format.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 
 using namespace llvm;
 
-/// Determine if a feature has a flag; '+' or '-'
-static inline bool hasFlag(StringRef Feature) {
-  assert(!Feature.empty() && "Empty string");
-  // Get first character
-  char Ch = Feature[0];
-  // Check if first character is '+' or '-' flag
-  return Ch == '+' || Ch =='-';
-}
-
-/// Return string stripped of flag.
-static inline std::string StripFlag(StringRef Feature) {
-  return hasFlag(Feature) ? Feature.substr(1) : Feature;
-}
-
-/// Return true if enable flag; '+'.
-static inline bool isEnabled(StringRef Feature) {
-  assert(!Feature.empty() && "Empty string");
-  // Get first character
-  char Ch = Feature[0];
-  // Check if first character is '+' for enabled
-  return Ch == '+';
-}
-
 /// Splits a string of comma separated items in to a vector of strings.
-static void Split(std::vector<std::string> &V, StringRef S) {
+void SubtargetFeatures::Split(std::vector<std::string> &V, StringRef S) {
   SmallVector<StringRef, 3> Tmp;
   S.split(Tmp, ',', -1, false /* KeepEmpty */);
   V.assign(Tmp.begin(), Tmp.end());
@@ -69,48 +44,6 @@ void SubtargetFeatures::AddFeature(StringRef String, bool Enable) {
                                        : (Enable ? "+" : "-") + String.lower());
 }
 
-/// Find KV in array using binary search.
-static const SubtargetFeatureKV *Find(StringRef S,
-                                      ArrayRef<SubtargetFeatureKV> A) {
-  // Binary search the array
-  auto F = std::lower_bound(A.begin(), A.end(), S);
-  // If not found then return NULL
-  if (F == A.end() || StringRef(F->Key) != S) return nullptr;
-  // Return the found array item
-  return F;
-}
-
-/// Return the length of the longest entry in the table.
-static size_t getLongestEntryLength(ArrayRef<SubtargetFeatureKV> Table) {
-  size_t MaxLen = 0;
-  for (auto &I : Table)
-    MaxLen = std::max(MaxLen, std::strlen(I.Key));
-  return MaxLen;
-}
-
-/// Display help for feature choices.
-static void Help(ArrayRef<SubtargetFeatureKV> CPUTable,
-                 ArrayRef<SubtargetFeatureKV> FeatTable) {
-  // Determine the length of the longest CPU and Feature entries.
-  unsigned MaxCPULen  = getLongestEntryLength(CPUTable);
-  unsigned MaxFeatLen = getLongestEntryLength(FeatTable);
-
-  // Print the CPU table.
-  errs() << "Available CPUs for this target:\n\n";
-  for (auto &CPU : CPUTable)
-    errs() << format("  %-*s - %s.\n", MaxCPULen, CPU.Key, CPU.Desc);
-  errs() << '\n';
-
-  // Print the Feature table.
-  errs() << "Available features for this target:\n\n";
-  for (auto &Feature : FeatTable)
-    errs() << format("  %-*s - %s.\n", MaxFeatLen, Feature.Key, Feature.Desc);
-  errs() << '\n';
-
-  errs() << "Use +feature to enable a feature, or -feature to disable it.\n"
-            "For example, llc -mcpu=mycpu -mattr=+feature1,-feature2\n";
-}
-
 SubtargetFeatures::SubtargetFeatures(StringRef Initial) {
   // Break up string into separate features
   Split(Features, Initial);
@@ -120,125 +53,6 @@ std::string SubtargetFeatures::getString() const {
   return join(Features.begin(), Features.end(), ",");
 }
 
-/// For each feature that is (transitively) implied by this feature, set it.
-static
-void SetImpliedBits(FeatureBitset &Bits, const FeatureBitset &Implies,
-                    ArrayRef<SubtargetFeatureKV> FeatureTable) {
-  // OR the Implies bits in outside the loop. This allows the Implies for CPUs
-  // which might imply features not in FeatureTable to use this.
-  Bits |= Implies;
-  for (const SubtargetFeatureKV &FE : FeatureTable)
-    if (Implies.test(FE.Value))
-      SetImpliedBits(Bits, FE.Implies.getAsBitset(), FeatureTable);
-}
-
-/// For each feature that (transitively) implies this feature, clear it.
-static
-void ClearImpliedBits(FeatureBitset &Bits, unsigned Value,
-                      ArrayRef<SubtargetFeatureKV> FeatureTable) {
-  for (const SubtargetFeatureKV &FE : FeatureTable) {
-    if (FE.Implies.getAsBitset().test(Value)) {
-      Bits.reset(FE.Value);
-      ClearImpliedBits(Bits, FE.Value, FeatureTable);
-    }
-  }
-}
-
-void
-SubtargetFeatures::ToggleFeature(FeatureBitset &Bits, StringRef Feature,
-                                 ArrayRef<SubtargetFeatureKV> FeatureTable) {
-  // Find feature in table.
-  const SubtargetFeatureKV *FeatureEntry =
-      Find(StripFlag(Feature), FeatureTable);
-  // If there is a match
-  if (FeatureEntry) {
-    if (Bits.test(FeatureEntry->Value)) {
-      Bits.reset(FeatureEntry->Value);
-      // For each feature that implies this, clear it.
-      ClearImpliedBits(Bits, FeatureEntry->Value, FeatureTable);
-    } else {
-      Bits.set(FeatureEntry->Value);
-
-      // For each feature that this implies, set it.
-      SetImpliedBits(Bits, FeatureEntry->Implies.getAsBitset(), FeatureTable);
-    }
-  } else {
-    errs() << "'" << Feature << "' is not a recognized feature for this target"
-           << " (ignoring feature)\n";
-  }
-}
-
-void SubtargetFeatures::ApplyFeatureFlag(FeatureBitset &Bits, StringRef Feature,
-                                    ArrayRef<SubtargetFeatureKV> FeatureTable) {
-  assert(hasFlag(Feature));
-
-  // Find feature in table.
-  const SubtargetFeatureKV *FeatureEntry =
-      Find(StripFlag(Feature), FeatureTable);
-  // If there is a match
-  if (FeatureEntry) {
-    // Enable/disable feature in bits
-    if (isEnabled(Feature)) {
-      Bits.set(FeatureEntry->Value);
-
-      // For each feature that this implies, set it.
-      SetImpliedBits(Bits, FeatureEntry->Implies.getAsBitset(), FeatureTable);
-    } else {
-      Bits.reset(FeatureEntry->Value);
-
-      // For each feature that implies this, clear it.
-      ClearImpliedBits(Bits, FeatureEntry->Value, FeatureTable);
-    }
-  } else {
-    errs() << "'" << Feature << "' is not a recognized feature for this target"
-           << " (ignoring feature)\n";
-  }
-}
-
-FeatureBitset
-SubtargetFeatures::getFeatureBits(StringRef CPU,
-                                  ArrayRef<SubtargetFeatureKV> CPUTable,
-                                  ArrayRef<SubtargetFeatureKV> FeatureTable) {
-  if (CPUTable.empty() || FeatureTable.empty())
-    return FeatureBitset();
-
-  assert(std::is_sorted(std::begin(CPUTable), std::end(CPUTable)) &&
-         "CPU table is not sorted");
-  assert(std::is_sorted(std::begin(FeatureTable), std::end(FeatureTable)) &&
-         "CPU features table is not sorted");
-  // Resulting bits
-  FeatureBitset Bits;
-
-  // Check if help is needed
-  if (CPU == "help")
-    Help(CPUTable, FeatureTable);
-
-  // Find CPU entry if CPU name is specified.
-  else if (!CPU.empty()) {
-    const SubtargetFeatureKV *CPUEntry = Find(CPU, CPUTable);
-
-    // If there is a match
-    if (CPUEntry) {
-      // Set the features implied by this CPU feature, if any.
-      SetImpliedBits(Bits, CPUEntry->Implies.getAsBitset(), FeatureTable);
-    } else {
-      errs() << "'" << CPU << "' is not a recognized processor for this target"
-             << " (ignoring processor)\n";
-    }
-  }
-
-  // Iterate through each feature
-  for (const std::string &Feature : Features) {
-    // Check for help
-    if (Feature == "+help")
-      Help(CPUTable, FeatureTable);
-    else
-      ApplyFeatureFlag(Bits, Feature, FeatureTable);
-  }
-
-  return Bits;
-}
-
 void SubtargetFeatures::print(raw_ostream &OS) const {
   for (auto &F : Features)
     OS << F << " ";