[PATCH] D145874: TableGen: Let getAllDerivedDefinitions() numeric order.

[NAKAMURA Takumi via Phabricator via llvm-commits]

chapuni created this revision.
Herald added subscribers: mgrang, hiraditya.
Herald added a project: All.
chapuni requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Since `RK::Recs` is sorted by character order, anonymous defs will be
enumerated like this;

- anonymous_0
- anonymous_1
- anonymous_10
- anonymous_100
- anonymous_1000
- ...
- anonymous_99
- anonymous_990
- ...
- anonymous_999

Some records around each gap might be wrapped around along increase or
decrease of records in middle. Then output order of anonymous defs
might be changed.

Numeric sort is expected to prevent such wrap-arounds.
This can be implemented with `StringRef::compare_numeric()`.

- ...
- anonymous_99
- anonymous_100
- ...
- anonymous_999
- anonymous_1000
- ...

I have optionally suppressed numeric sort on `RegisterTuples`
in `CodeGenRegBank`, or several tests would fail due to behavior change.

Although it would be simpler to change the comparator of `Recs` as numeric,
I don't do, since;

- I am afraid if numeric comparator is not lightweit.
- It triggers behavior change (mentioned above)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145874

Files:
  llvm/include/llvm/TableGen/Record.h
  llvm/lib/TableGen/Record.cpp
  llvm/utils/TableGen/CodeGenRegisters.cpp


Index: llvm/utils/TableGen/CodeGenRegisters.cpp
===================================================================
--- llvm/utils/TableGen/CodeGenRegisters.cpp
+++ llvm/utils/TableGen/CodeGenRegisters.cpp
@@ -1174,8 +1174,8 @@
     getReg(Regs[i]);
 
   // Expand tuples and number the new registers.
-  std::vector<Record*> Tups =
-    Records.getAllDerivedDefinitions("RegisterTuples");
+  std::vector<Record *> Tups =
+      Records.getAllDerivedDefinitions("RegisterTuples", false);
 
   for (Record *R : Tups) {
     std::vector<Record *> TupRegs = *Sets.expand(R);
Index: llvm/lib/TableGen/Record.cpp
===================================================================
--- llvm/lib/TableGen/Record.cpp
+++ llvm/lib/TableGen/Record.cpp
@@ -2976,18 +2976,21 @@
 }
 
 std::vector<Record *>
-RecordKeeper::getAllDerivedDefinitions(StringRef ClassName) const {
+RecordKeeper::getAllDerivedDefinitions(StringRef ClassName,
+                                       bool SortNumeric) const {
   // We cache the record vectors for single classes. Many backends request
   // the same vectors multiple times.
   auto Pair = ClassRecordsMap.try_emplace(ClassName);
   if (Pair.second)
-    Pair.first->second = getAllDerivedDefinitions(ArrayRef(ClassName));
+    Pair.first->second =
+        getAllDerivedDefinitions(ArrayRef(ClassName), SortNumeric);
 
   return Pair.first->second;
 }
 
-std::vector<Record *> RecordKeeper::getAllDerivedDefinitions(
-    ArrayRef<StringRef> ClassNames) const {
+std::vector<Record *>
+RecordKeeper::getAllDerivedDefinitions(ArrayRef<StringRef> ClassNames,
+                                       bool SortNumeric) const {
   SmallVector<Record *, 2> ClassRecs;
   std::vector<Record *> Defs;
 
@@ -3006,12 +3009,18 @@
       Defs.push_back(OneDef.second.get());
   }
 
+  if (SortNumeric)
+    llvm::sort(Defs, [](Record *LHS, Record *RHS) {
+      return LHS->getName().compare_numeric(RHS->getName()) < 0;
+    });
+
   return Defs;
 }
 
 std::vector<Record *>
-RecordKeeper::getAllDerivedDefinitionsIfDefined(StringRef ClassName) const {
-  return getClass(ClassName) ? getAllDerivedDefinitions(ClassName)
+RecordKeeper::getAllDerivedDefinitionsIfDefined(StringRef ClassName,
+                                                bool SortNumeric) const {
+  return getClass(ClassName) ? getAllDerivedDefinitions(ClassName, SortNumeric)
                              : std::vector<Record *>();
 }
 
Index: llvm/include/llvm/TableGen/Record.h
===================================================================
--- llvm/include/llvm/TableGen/Record.h
+++ llvm/include/llvm/TableGen/Record.h
@@ -1999,17 +1999,19 @@
 
   /// Get all the concrete records that inherit from the one specified
   /// class. The class must be defined.
-  std::vector<Record *> getAllDerivedDefinitions(StringRef ClassName) const;
+  std::vector<Record *> getAllDerivedDefinitions(StringRef ClassName,
+                                                 bool SortNumeric = true) const;
 
   /// Get all the concrete records that inherit from all the specified
   /// classes. The classes must be defined.
-  std::vector<Record *> getAllDerivedDefinitions(
-      ArrayRef<StringRef> ClassNames) const;
+  std::vector<Record *> getAllDerivedDefinitions(ArrayRef<StringRef> ClassNames,
+                                                 bool SortNumeric = true) const;
 
   /// 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.
   std::vector<Record *>
-  getAllDerivedDefinitionsIfDefined(StringRef ClassName) const;
+  getAllDerivedDefinitionsIfDefined(StringRef ClassName,
+                                    bool SortNumeric = true) const;
 
   void dump() const;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D145874.504438.patch
Type: text/x-patch
Size: 3771 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230312/1db661d3/attachment.bin>