Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TableGen] Delete non-const overloads of getAllDerivedDefinitions #110990

Merged

Conversation

jurahul
Copy link
Contributor

@jurahul jurahul commented Oct 3, 2024

Now that all TableGen backends have transitioned to const versions of these functions, we do not need the non-const versions anymore.

This is a part of effort to have better const correctness in TableGen backends:

https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089

Now that all TableGen backends have transitioned to const versions
of these functions, we do not need the non-const versions anymore.
@jurahul jurahul marked this pull request as ready for review October 3, 2024 14:01
@jurahul jurahul requested a review from arsenm October 3, 2024 14:01
@llvmbot
Copy link
Member

llvmbot commented Oct 3, 2024

@llvm/pr-subscribers-tablegen

Author: Rahul Joshi (jurahul)

Changes

Now that all TableGen backends have transitioned to const versions of these functions, we do not need the non-const versions anymore.


Full diff: https://github.com/llvm/llvm-project/pull/110990.diff

2 Files Affected:

  • (modified) llvm/include/llvm/TableGen/Record.h (+1-25)
  • (modified) llvm/lib/TableGen/Record.cpp (+12-51)
diff --git a/llvm/include/llvm/TableGen/Record.h b/llvm/include/llvm/TableGen/Record.h
index 106fee39cb9f64..fc62b98b841897 100644
--- a/llvm/include/llvm/TableGen/Record.h
+++ b/llvm/include/llvm/TableGen/Record.h
@@ -2058,28 +2058,19 @@ class RecordKeeper {
   //===--------------------------------------------------------------------===//
   // High-level helper methods, useful for tablegen backends.
 
-  // Non-const methods return std::vector<Record *> by value or reference.
-  // Const methods return std::vector<const Record *> by value or
-  // ArrayRef<const Record *>.
-
   /// Get all the concrete records that inherit from the one specified
   /// class. The class must be defined.
   ArrayRef<const Record *> getAllDerivedDefinitions(StringRef ClassName) const;
-  const std::vector<Record *> &getAllDerivedDefinitions(StringRef ClassName);
 
   /// Get all the concrete records that inherit from all the specified
   /// classes. The classes must be defined.
   std::vector<const Record *>
   getAllDerivedDefinitions(ArrayRef<StringRef> ClassNames) const;
-  std::vector<Record *>
-  getAllDerivedDefinitions(ArrayRef<StringRef> ClassNames);
 
   /// Get all the concrete records that inherit from specified class, if the
   /// class is defined. Returns an empty vector if the class is not defined.
   ArrayRef<const Record *>
   getAllDerivedDefinitionsIfDefined(StringRef ClassName) const;
-  const std::vector<Record *> &
-  getAllDerivedDefinitionsIfDefined(StringRef ClassName);
 
   void dump() const;
 
@@ -2091,24 +2082,9 @@ class RecordKeeper {
   RecordKeeper &operator=(RecordKeeper &&) = delete;
   RecordKeeper &operator=(const RecordKeeper &) = delete;
 
-  // Helper template functions for backend accessors.
-  template <typename VecTy>
-  const VecTy &
-  getAllDerivedDefinitionsImpl(StringRef ClassName,
-                               std::map<std::string, VecTy> &Cache) const;
-
-  template <typename VecTy>
-  VecTy getAllDerivedDefinitionsImpl(ArrayRef<StringRef> ClassNames) const;
-
-  template <typename VecTy>
-  const VecTy &getAllDerivedDefinitionsIfDefinedImpl(
-      StringRef ClassName, std::map<std::string, VecTy> &Cache) const;
-
   std::string InputFilename;
   RecordMap Classes, Defs;
-  mutable std::map<std::string, std::vector<const Record *>>
-      ClassRecordsMapConst;
-  mutable std::map<std::string, std::vector<Record *>> ClassRecordsMap;
+  mutable std::map<std::string, std::vector<const Record *>> Cache;
   GlobalMap ExtraGlobals;
 
   // TODO: Move timing related code out of RecordKeeper.
diff --git a/llvm/lib/TableGen/Record.cpp b/llvm/lib/TableGen/Record.cpp
index c0c89836171b12..315ea177098efd 100644
--- a/llvm/lib/TableGen/Record.cpp
+++ b/llvm/lib/TableGen/Record.cpp
@@ -3275,24 +3275,20 @@ void RecordKeeper::stopBackendTimer() {
   }
 }
 
-template <typename VecTy>
-const VecTy &RecordKeeper::getAllDerivedDefinitionsImpl(
-    StringRef ClassName, std::map<std::string, VecTy> &Cache) const {
+ArrayRef<const Record *>
+RecordKeeper::getAllDerivedDefinitions(StringRef ClassName) const {
   // We cache the record vectors for single classes. Many backends request
   // the same vectors multiple times.
-  auto Pair = Cache.try_emplace(ClassName.str());
-  if (Pair.second)
-    Pair.first->second =
-        getAllDerivedDefinitionsImpl<VecTy>(ArrayRef(ClassName));
-
-  return Pair.first->second;
+  auto [Iter, Inserted] = Cache.try_emplace(ClassName.str());
+  if (Inserted)
+    Iter->second = getAllDerivedDefinitions(ArrayRef(ClassName));
+  return Iter->second;
 }
 
-template <typename VecTy>
-VecTy RecordKeeper::getAllDerivedDefinitionsImpl(
-    ArrayRef<StringRef> ClassNames) const {
+std::vector<const Record *>
+RecordKeeper::getAllDerivedDefinitions(ArrayRef<StringRef> ClassNames) const {
   SmallVector<const Record *, 2> ClassRecs;
-  VecTy Defs;
+  std::vector<const Record *> Defs;
 
   assert(ClassNames.size() > 0 && "At least one class must be passed.");
   for (const auto &ClassName : ClassNames) {
@@ -3312,46 +3308,11 @@ VecTy RecordKeeper::getAllDerivedDefinitionsImpl(
   return Defs;
 }
 
-template <typename VecTy>
-const VecTy &RecordKeeper::getAllDerivedDefinitionsIfDefinedImpl(
-    StringRef ClassName, std::map<std::string, VecTy> &Cache) const {
-  return getClass(ClassName)
-             ? getAllDerivedDefinitionsImpl<VecTy>(ClassName, Cache)
-             : Cache[""];
-}
-
-ArrayRef<const Record *>
-RecordKeeper::getAllDerivedDefinitions(StringRef ClassName) const {
-  return getAllDerivedDefinitionsImpl<std::vector<const Record *>>(
-      ClassName, ClassRecordsMapConst);
-}
-
-const std::vector<Record *> &
-RecordKeeper::getAllDerivedDefinitions(StringRef ClassName) {
-  return getAllDerivedDefinitionsImpl<std::vector<Record *>>(ClassName,
-                                                             ClassRecordsMap);
-}
-
-std::vector<const Record *>
-RecordKeeper::getAllDerivedDefinitions(ArrayRef<StringRef> ClassNames) const {
-  return getAllDerivedDefinitionsImpl<std::vector<const Record *>>(ClassNames);
-}
-
-std::vector<Record *>
-RecordKeeper::getAllDerivedDefinitions(ArrayRef<StringRef> ClassNames) {
-  return getAllDerivedDefinitionsImpl<std::vector<Record *>>(ClassNames);
-}
-
 ArrayRef<const Record *>
 RecordKeeper::getAllDerivedDefinitionsIfDefined(StringRef ClassName) const {
-  return getAllDerivedDefinitionsIfDefinedImpl<std::vector<const Record *>>(
-      ClassName, ClassRecordsMapConst);
-}
-
-const std::vector<Record *> &
-RecordKeeper::getAllDerivedDefinitionsIfDefined(StringRef ClassName) {
-  return getAllDerivedDefinitionsIfDefinedImpl<std::vector<Record *>>(
-      ClassName, ClassRecordsMap);
+  if (getClass(ClassName))
+    return getAllDerivedDefinitions(ClassName);
+  return Cache[""];
 }
 
 void RecordKeeper::dumpAllocationStats(raw_ostream &OS) const {

@jurahul jurahul merged commit b78bfe7 into llvm:main Oct 3, 2024
12 checks passed
@jurahul jurahul deleted the delete_non_const_get_all_derived_definitions branch October 3, 2024 15:18
xgupta pushed a commit to xgupta/llvm-project that referenced this pull request Oct 4, 2024
…lvm#110990)

Now that all TableGen backends have transitioned to const versions of
these functions, we do not need the non-const versions anymore.

This is a part of effort to have better const correctness in TableGen
backends:


https://discourse.llvm.org/t/psa-planned-changes-to-tablegen-getallderiveddefinitions-api-potential-downstream-breakages/81089
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants