[llvm] 4faf3aa - Revert "[CodeGen] Fix incorrect usage of MCPhysReg for diff list elements"

Sergei Barannikov via llvm-commits llvm-commits at lists.llvm.org
Mon May 22 19:15:08 PDT 2023


Author: Sergei Barannikov
Date: 2023-05-23T05:14:34+03:00
New Revision: 4faf3aaf28226a4e950c103a14f6fc1d1fdabb1b

URL: https://github.com/llvm/llvm-project/commit/4faf3aaf28226a4e950c103a14f6fc1d1fdabb1b
DIFF: https://github.com/llvm/llvm-project/commit/4faf3aaf28226a4e950c103a14f6fc1d1fdabb1b.diff

LOG: Revert "[CodeGen] Fix incorrect usage of MCPhysReg for diff list elements"

This reverts commit fa2827f0796c08e36b0b157fc526dd59cd6368e3.

Causes build bot failres:
https://lab.llvm.org/buildbot/#/builders/38/builds/12037

Added: 
    

Modified: 
    llvm/include/llvm/MC/MCRegisterInfo.h
    llvm/utils/TableGen/RegisterInfoEmitter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/MC/MCRegisterInfo.h b/llvm/include/llvm/MC/MCRegisterInfo.h
index 233c7feb82a57..cafa0ba4913fc 100644
--- a/llvm/include/llvm/MC/MCRegisterInfo.h
+++ b/llvm/include/llvm/MC/MCRegisterInfo.h
@@ -111,8 +111,8 @@ struct MCRegisterDesc {
   // sub-register in SubRegs.
   uint32_t SubRegIndices;
 
-  // Points to the list of register units. The low bits hold the first regunit
-  // number, the high bits hold an offset into DiffLists. See MCRegUnitIterator.
+  // RegUnits - Points to the list of register units. The low 4 bits holds the
+  // Scale, the high bits hold an offset into DiffLists. See MCRegUnitIterator.
   uint32_t RegUnits;
 
   /// Index into list with lane mask sequences. The sequence contains a lanemask
@@ -161,7 +161,7 @@ class MCRegisterInfo {
   unsigned NumClasses;                        // Number of entries in the array
   unsigned NumRegUnits;                       // Number of regunits.
   const MCPhysReg (*RegUnitRoots)[2];         // Pointer to regunit root table.
-  const int16_t *DiffLists;                   // Pointer to the 
diff lists array
+  const MCPhysReg *DiffLists;                 // Pointer to the 
diff lists array
   const LaneBitmask *RegUnitMaskSequences;    // Pointer to lane mask sequences
                                               // for register units.
   const char *RegStrings;                     // Pointer to the string table.
@@ -194,19 +194,31 @@ class MCRegisterInfo {
   /// Don't use this class directly, use one of the specialized sub-classes
   /// defined below.
   class DiffListIterator {
-    unsigned Val = 0;
-    const int16_t *List = nullptr;
+    uint16_t Val = 0;
+    const MCPhysReg *List = nullptr;
 
   protected:
     /// Create an invalid iterator. Call init() to point to something useful.
     DiffListIterator() = default;
 
-    /// Point the iterator to InitVal, decoding subsequent values from DiffList.
-    void init(unsigned InitVal, const int16_t *DiffList) {
+    /// init - Point the iterator to InitVal, decoding subsequent values from
+    /// DiffList. The iterator will initially point to InitVal, sub-classes are
+    /// responsible for skipping the seed value if it is not part of the list.
+    void init(MCPhysReg InitVal, const MCPhysReg *DiffList) {
       Val = InitVal;
       List = DiffList;
     }
 
+    /// advance - Move to the next list position, return the applied
+    /// 
diff erential. This function does not detect the end of the list, that
+    /// is the caller's responsibility (by checking for a 0 return value).
+    MCRegister advance() {
+      assert(isValid() && "Cannot move off the end of the list.");
+      MCPhysReg D = *List++;
+      Val += D;
+      return D;
+    }
+
   public:
     /// isValid - returns true if this iterator is not yet at the end.
     bool isValid() const { return List; }
@@ -216,11 +228,8 @@ class MCRegisterInfo {
 
     /// Pre-increment to move to the next position.
     void operator++() {
-      assert(isValid() && "Cannot move off the end of the list.");
-      int16_t D = *List++;
-      Val += D;
       // The end of the list is encoded as a 0 
diff erential.
-      if (!D)
+      if (!advance())
         List = nullptr;
     }
 
@@ -240,8 +249,8 @@ class MCRegisterInfo {
     mc_
diff list_iterator(MCRegisterInfo::DiffListIterator Iter) : Iter(Iter) {}
 
     // Allow conversion between instantiations where valid.
-    mc_
diff list_iterator(unsigned InitVal, const int16_t *DiffList) {
-      Iter.init(InitVal, DiffList);
+    mc_
diff list_iterator(MCRegister Reg, const MCPhysReg *DiffList) {
+      Iter.init(Reg, DiffList);
       Val = *Iter;
     }
 
@@ -342,11 +351,16 @@ class MCRegisterInfo {
   /// Initialize MCRegisterInfo, called by TableGen
   /// auto-generated routines. *DO NOT USE*.
   void InitMCRegisterInfo(const MCRegisterDesc *D, unsigned NR, unsigned RA,
-                          unsigned PC, const MCRegisterClass *C, unsigned NC,
-                          const MCPhysReg (*RURoots)[2], unsigned NRU,
-                          const int16_t *DL, const LaneBitmask *RUMS,
-                          const char *Strings, const char *ClassStrings,
-                          const uint16_t *SubIndices, unsigned NumIndices,
+                          unsigned PC,
+                          const MCRegisterClass *C, unsigned NC,
+                          const MCPhysReg (*RURoots)[2],
+                          unsigned NRU,
+                          const MCPhysReg *DL,
+                          const LaneBitmask *RUMS,
+                          const char *Strings,
+                          const char *ClassStrings,
+                          const uint16_t *SubIndices,
+                          unsigned NumIndices,
                           const SubRegCoveredBits *SubIdxRanges,
                           const uint16_t *RET) {
     Desc = D;
@@ -661,9 +675,6 @@ inline bool MCRegisterInfo::isSuperRegister(MCRegister RegA, MCRegister RegB) co
 // MCRegUnitIterator enumerates a list of register units for Reg. The list is
 // in ascending numerical order.
 class MCRegUnitIterator : public MCRegisterInfo::DiffListIterator {
-  // The value must be kept in sync with RegisterInfoEmitter.cpp.
-  static constexpr unsigned RegUnitBits = 12;
-
 public:
   /// MCRegUnitIterator - Create an iterator that traverses the register units
   /// in Reg.
@@ -674,9 +685,18 @@ class MCRegUnitIterator : public MCRegisterInfo::DiffListIterator {
     assert(MCRegister::isPhysicalRegister(Reg.id()));
     // Decode the RegUnits MCRegisterDesc field.
     unsigned RU = MCRI->get(Reg).RegUnits;
-    unsigned FirstRU = RU & ((1u << RegUnitBits) - 1);
-    unsigned Offset = RU >> RegUnitBits;
-    init(FirstRU, MCRI->DiffLists + Offset);
+    unsigned Scale = RU & 15;
+    unsigned Offset = RU >> 4;
+
+    // Initialize the iterator to Reg * Scale, and the List pointer to
+    // DiffLists + Offset.
+    init(Reg * Scale, MCRI->DiffLists + Offset);
+
+    // That may not be a valid unit, we need to advance by one to get the real
+    // unit number. The first 
diff erential can be 0 which would normally
+    // terminate the list, but since we know every register has at least one
+    // unit, we can allow a 0 
diff erential here.
+    advance();
   }
 
   MCRegUnitIterator &operator++() {

diff  --git a/llvm/utils/TableGen/RegisterInfoEmitter.cpp b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
index 1f433c01dca5f..f35dd368cf667 100644
--- a/llvm/utils/TableGen/RegisterInfoEmitter.cpp
+++ b/llvm/utils/TableGen/RegisterInfoEmitter.cpp
@@ -635,16 +635,17 @@ static void printSubRegIndex(raw_ostream &OS, const CodeGenSubRegIndex *Idx) {
 // The initial value depends on the specific list. The list is terminated by a
 // 0 
diff erential which means we can't encode repeated elements.
 
-typedef SmallVector<int16_t, 4> DiffVec;
+typedef SmallVector<uint16_t, 4> DiffVec;
 typedef SmallVector<LaneBitmask, 4> MaskVec;
 
-// Fills V with 
diff erentials between every two consecutive elements of List.
-static DiffVec &
diff Encode(DiffVec &V, SparseBitVector<> List) {
+// Differentially encode a sequence of numbers into V. The starting value and
+// terminating 0 are not added to V, so it will have the same size as List.
+static
+DiffVec &
diff Encode(DiffVec &V, unsigned InitVal, SparseBitVector<> List) {
   assert(V.empty() && "Clear DiffVec before 
diff Encode.");
-  SparseBitVector<>::iterator I = List.begin(), E = List.end();
-  unsigned Val = *I;
-  while (++I != E) {
-    unsigned Cur = *I;
+  uint16_t Val = uint16_t(InitVal);
+
+  for (uint16_t Cur : List) {
     V.push_back(Cur - Val);
     Val = Cur;
   }
@@ -655,16 +656,18 @@ template<typename Iter>
 static
 DiffVec &
diff Encode(DiffVec &V, unsigned InitVal, Iter Begin, Iter End) {
   assert(V.empty() && "Clear DiffVec before 
diff Encode.");
-  unsigned Val = InitVal;
+  uint16_t Val = uint16_t(InitVal);
   for (Iter I = Begin; I != End; ++I) {
-    unsigned Cur = (*I)->EnumValue;
+    uint16_t Cur = (*I)->EnumValue;
     V.push_back(Cur - Val);
     Val = Cur;
   }
   return V;
 }
 
-static void printDiff16(raw_ostream &OS, int16_t Val) { OS << Val; }
+static void printDiff16(raw_ostream &OS, uint16_t Val) {
+  OS << Val;
+}
 
 static void printMask(raw_ostream &OS, LaneBitmask Val) {
   OS << "LaneBitmask(0x" << PrintLaneMask(Val) << ')';
@@ -888,6 +891,7 @@ RegisterInfoEmitter::runMCDesc(raw_ostream &OS, CodeGenTarget &Target,
   SmallVector<DiffVec, 4> SubRegLists(Regs.size());
   SmallVector<DiffVec, 4> SuperRegLists(Regs.size());
   SmallVector<DiffVec, 4> RegUnitLists(Regs.size());
+  SmallVector<unsigned, 4> RegUnitInitScale(Regs.size());
 
   // List of lane masks accompanying register unit sequences.
   SequenceToOffsetTable<MaskVec> LaneMaskSeqs;
@@ -925,8 +929,31 @@ RegisterInfoEmitter::runMCDesc(raw_ostream &OS, CodeGenTarget &Target,
                SuperRegList.end());
     DiffSeqs.add(SuperRegLists[i]);
 
-    const SparseBitVector<> &RUs = Reg.getNativeRegUnits();
-    DiffSeqs.add(
diff Encode(RegUnitLists[i], RUs));
+    // Differentially encode the register unit list, seeded by register number.
+    // First compute a scale factor that allows more 
diff -lists to be reused:
+    //
+    //   D0 -> (S0, S1)
+    //   D1 -> (S2, S3)
+    //
+    // A scale factor of 2 allows D0 and D1 to share a 
diff -list. The initial
+    // value for the 
diff erential decoder is the register number multiplied by
+    // the scale.
+    //
+    // Check the neighboring registers for arithmetic progressions.
+    unsigned ScaleA = ~0u, ScaleB = ~0u;
+    SparseBitVector<> RUs = Reg.getNativeRegUnits();
+    if (I != Regs.begin() &&
+        std::prev(I)->getNativeRegUnits().count() == RUs.count())
+      ScaleB = *RUs.begin() - *std::prev(I)->getNativeRegUnits().begin();
+    if (std::next(I) != Regs.end() &&
+        std::next(I)->getNativeRegUnits().count() == RUs.count())
+      ScaleA = *std::next(I)->getNativeRegUnits().begin() - *RUs.begin();
+    unsigned Scale = std::min(ScaleB, ScaleA);
+    // Default the scale to 0 if it can't be encoded in 4 bits.
+    if (Scale >= 16)
+      Scale = 0;
+    RegUnitInitScale[i] = Scale;
+    DiffSeqs.add(
diff Encode(RegUnitLists[i], Scale * Reg.EnumValue, RUs));
 
     const auto &RUMasks = Reg.getRegUnitLaneMasks();
     MaskVec &LaneMaskVec = RegUnitLaneMasks[i];
@@ -951,7 +978,7 @@ RegisterInfoEmitter::runMCDesc(raw_ostream &OS, CodeGenTarget &Target,
   const std::string &TargetName = std::string(Target.getName());
 
   // Emit the shared table of 
diff erential lists.
-  OS << "extern const int16_t " << TargetName << "RegDiffLists[] = {\n";
+  OS << "extern const MCPhysReg " << TargetName << "RegDiffLists[] = {\n";
   DiffSeqs.emit(OS, printDiff16);
   OS << "};\n\n";
 
@@ -987,16 +1014,10 @@ RegisterInfoEmitter::runMCDesc(raw_ostream &OS, CodeGenTarget &Target,
   // Emit the register descriptors now.
   i = 0;
   for (const auto &Reg : Regs) {
-    unsigned FirstRU = Reg.getNativeRegUnits().find_first();
-    unsigned Offset = DiffSeqs.get(RegUnitLists[i]);
-    // The value must be kept in sync with MCRegisterInfo.h.
-    constexpr unsigned RegUnitBits = 12;
-    assert(isUInt<RegUnitBits>(FirstRU) && "Too many regunits");
-    assert(isUInt<32 - RegUnitBits>(Offset) && "Offset is too big");
     OS << "  { " << RegStrings.get(std::string(Reg.getName())) << ", "
        << DiffSeqs.get(SubRegLists[i]) << ", " << DiffSeqs.get(SuperRegLists[i])
        << ", " << SubRegIdxSeqs.get(SubRegIdxLists[i]) << ", "
-       << (Offset << RegUnitBits | FirstRU) << ", "
+       << (DiffSeqs.get(RegUnitLists[i]) * 16 + RegUnitInitScale[i]) << ", "
        << LaneMaskSeqs.get(RegUnitLaneMasks[i]) << " },\n";
     ++i;
   }
@@ -1630,7 +1651,7 @@ RegisterInfoEmitter::runTargetDesc(raw_ostream &OS, CodeGenTarget &Target,
 
   // Emit the constructor of the class...
   OS << "extern const MCRegisterDesc " << TargetName << "RegDesc[];\n";
-  OS << "extern const int16_t " << TargetName << "RegDiffLists[];\n";
+  OS << "extern const MCPhysReg " << TargetName << "RegDiffLists[];\n";
   OS << "extern const LaneBitmask " << TargetName << "LaneMaskLists[];\n";
   OS << "extern const char " << TargetName << "RegStrings[];\n";
   OS << "extern const char " << TargetName << "RegClassStrings[];\n";


        


More information about the llvm-commits mailing list