[llvm] [TableGen] Remove redundant buffer copies for ULEB128 decode calls. (PR #80199)

Jason Eckhardt via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 3 16:09:44 PST 2024


https://github.com/nvjle updated https://github.com/llvm/llvm-project/pull/80199

>From e8bdcf9e3c7b18ea95bce4b87ec80a23175ffaf3 Mon Sep 17 00:00:00 2001
From: Jason Eckhardt <jeckhardt at nvidia.com>
Date: Wed, 31 Jan 2024 15:07:32 -0600
Subject: [PATCH 1/2] [TableGen] Remove redundant buffer copies for ULEB128
 decode calls.

This patch removes a couple of redundant buffer copies in emitTable
for setting up calls to decodeULEB128. Instead, provide the Table.data
buffer directly to the calls-- where decodeULEB128 does its own buffer
overflow checking.

Factor out 7 explicit loops to emit ULEB128 bytes into emitULEB128.

The changed functionality is already covered by existing unit tests
and by virtue of most of the in-tree back-ends exercising the decoder
emitter.
---
 llvm/utils/TableGen/DecoderEmitter.cpp | 61 ++++++++++++--------------
 1 file changed, 29 insertions(+), 32 deletions(-)

diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index 525aae02ded90..a5abcba036d90 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -775,6 +775,18 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
 
   Indentation += 2;
 
+  // Emit ULEB128 encoded value to OS, returning the number of bytes emitted.
+  auto emitULEB128 = [](DecoderTable::const_iterator I,
+                        formatted_raw_ostream &OS) {
+    unsigned Len = 0;
+    while (*I >= 128) {
+      OS << (unsigned)*I++ << ", ";
+      Len++;
+    }
+    OS << (unsigned)*I++ << ", ";
+    return Len + 1;
+  };
+
   // FIXME: We may be able to use the NumToSkip values to recover
   // appropriate indentation levels.
   DecoderTable::const_iterator I = Table.begin();
@@ -794,14 +806,11 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
       OS.indent(Indentation) << "MCD::OPC_ExtractField, ";
 
       // ULEB128 encoded start value.
-      uint8_t Buffer[16], *P = Buffer;
-      while ((*P++ = *I++) >= 128)
-        assert((P - Buffer) <= (ptrdiff_t)sizeof(Buffer) &&
-               "ULEB128 value too large!");
-      unsigned Start = decodeULEB128(Buffer);
-      for (P = Buffer; *P >= 128; ++P)
-        OS << (unsigned)*P << ", ";
-      OS << (unsigned)*P << ", ";
+      const char *ErrMsg = nullptr;
+      unsigned Start = decodeULEB128(Table.data() + Pos + 1, nullptr,
+                                     Table.data() + Table.size(), &ErrMsg);
+      assert(ErrMsg == nullptr && "ULEB128 value too large!");
+      I += emitULEB128(I, OS);
 
       unsigned Len = *I++;
       OS << Len << ",  // Inst{";
@@ -814,9 +823,7 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
       ++I;
       OS.indent(Indentation) << "MCD::OPC_FilterValue, ";
       // The filter value is ULEB128 encoded.
-      while (*I >= 128)
-        OS << (unsigned)*I++ << ", ";
-      OS << (unsigned)*I++ << ", ";
+      I += emitULEB128(I, OS);
 
       // 24-bit numtoskip value.
       uint8_t Byte = *I++;
@@ -835,16 +842,13 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
       ++I;
       OS.indent(Indentation) << "MCD::OPC_CheckField, ";
       // ULEB128 encoded start value.
-      for (; *I >= 128; ++I)
-        OS << (unsigned)*I << ", ";
-      OS << (unsigned)*I++ << ", ";
+      I += emitULEB128(I, OS);
       // 8-bit length.
       unsigned Len = *I++;
       OS << Len << ", ";
       // ULEB128 encoded field value.
-      for (; *I >= 128; ++I)
-        OS << (unsigned)*I << ", ";
-      OS << (unsigned)*I++ << ", ";
+      I += emitULEB128(I, OS);
+
       // 24-bit numtoskip value.
       uint8_t Byte = *I++;
       uint32_t NumToSkip = Byte;
@@ -861,9 +865,7 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
     case MCD::OPC_CheckPredicate: {
       ++I;
       OS.indent(Indentation) << "MCD::OPC_CheckPredicate, ";
-      for (; *I >= 128; ++I)
-        OS << (unsigned)*I << ", ";
-      OS << (unsigned)*I++ << ", ";
+      I += emitULEB128(I, OS);
 
       // 24-bit numtoskip value.
       uint8_t Byte = *I++;
@@ -882,23 +884,18 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
     case MCD::OPC_TryDecode: {
       bool IsTry = *I == MCD::OPC_TryDecode;
       ++I;
-      // Extract the ULEB128 encoded Opcode to a buffer.
-      uint8_t Buffer[16], *p = Buffer;
-      while ((*p++ = *I++) >= 128)
-        assert((p - Buffer) <= (ptrdiff_t)sizeof(Buffer)
-               && "ULEB128 value too large!");
       // Decode the Opcode value.
-      unsigned Opc = decodeULEB128(Buffer);
+      const char *ErrMsg = nullptr;
+      unsigned Opc = decodeULEB128(Table.data() + Pos + 1, nullptr,
+                                   Table.data() + Table.size(), &ErrMsg);
+      assert(ErrMsg == nullptr && "ULEB128 value too large!");
+
       OS.indent(Indentation) << "MCD::OPC_" << (IsTry ? "Try" : "")
         << "Decode, ";
-      for (p = Buffer; *p >= 128; ++p)
-        OS << (unsigned)*p << ", ";
-      OS << (unsigned)*p << ", ";
+      I += emitULEB128(I, OS);
 
       // Decoder index.
-      for (; *I >= 128; ++I)
-        OS << (unsigned)*I << ", ";
-      OS << (unsigned)*I++ << ", ";
+      I += emitULEB128(I, OS);
 
       if (!IsTry) {
         OS << "// Opcode: " << NumberedEncodings[Opc] << "\n";

>From b507e9b044e2765cadd72d23b3a67c1e594ff2d6 Mon Sep 17 00:00:00 2001
From: Jason Eckhardt <jeckhardt at nvidia.com>
Date: Sat, 3 Feb 2024 18:08:05 -0600
Subject: [PATCH 2/2] Also factor out 4 copies of 24-bit numtoskip emission.

---
 llvm/utils/TableGen/DecoderEmitter.cpp | 59 ++++++++++----------------
 1 file changed, 23 insertions(+), 36 deletions(-)

diff --git a/llvm/utils/TableGen/DecoderEmitter.cpp b/llvm/utils/TableGen/DecoderEmitter.cpp
index a5abcba036d90..591ee5c728874 100644
--- a/llvm/utils/TableGen/DecoderEmitter.cpp
+++ b/llvm/utils/TableGen/DecoderEmitter.cpp
@@ -787,6 +787,21 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
     return Len + 1;
   };
 
+  // Emit 24-bit numtoskip value to OS, returning the NumToSkip value.
+  auto emitNumToSkip = [](DecoderTable::const_iterator I,
+                          formatted_raw_ostream &OS) {
+    uint8_t Byte = *I++;
+    uint32_t NumToSkip = Byte;
+    OS << (unsigned)Byte << ", ";
+    Byte = *I++;
+    OS << (unsigned)Byte << ", ";
+    NumToSkip |= Byte << 8;
+    Byte = *I++;
+    OS << utostr(Byte) << ", ";
+    NumToSkip |= Byte << 16;
+    return NumToSkip;
+  };
+
   // FIXME: We may be able to use the NumToSkip values to recover
   // appropriate indentation levels.
   DecoderTable::const_iterator I = Table.begin();
@@ -826,15 +841,8 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
       I += emitULEB128(I, OS);
 
       // 24-bit numtoskip value.
-      uint8_t Byte = *I++;
-      uint32_t NumToSkip = Byte;
-      OS << (unsigned)Byte << ", ";
-      Byte = *I++;
-      OS << (unsigned)Byte << ", ";
-      NumToSkip |= Byte << 8;
-      Byte = *I++;
-      OS << utostr(Byte) << ", ";
-      NumToSkip |= Byte << 16;
+      uint32_t NumToSkip = emitNumToSkip(I, OS);
+      I += 3;
       OS << "// Skip to: " << ((I - Table.begin()) + NumToSkip) << "\n";
       break;
     }
@@ -850,15 +858,8 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
       I += emitULEB128(I, OS);
 
       // 24-bit numtoskip value.
-      uint8_t Byte = *I++;
-      uint32_t NumToSkip = Byte;
-      OS << (unsigned)Byte << ", ";
-      Byte = *I++;
-      OS << (unsigned)Byte << ", ";
-      NumToSkip |= Byte << 8;
-      Byte = *I++;
-      OS << utostr(Byte) << ", ";
-      NumToSkip |= Byte << 16;
+      uint32_t NumToSkip = emitNumToSkip(I, OS);
+      I += 3;
       OS << "// Skip to: " << ((I - Table.begin()) + NumToSkip) << "\n";
       break;
     }
@@ -868,15 +869,8 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
       I += emitULEB128(I, OS);
 
       // 24-bit numtoskip value.
-      uint8_t Byte = *I++;
-      uint32_t NumToSkip = Byte;
-      OS << (unsigned)Byte << ", ";
-      Byte = *I++;
-      OS << (unsigned)Byte << ", ";
-      NumToSkip |= Byte << 8;
-      Byte = *I++;
-      OS << utostr(Byte) << ", ";
-      NumToSkip |= Byte << 16;
+      uint32_t NumToSkip = emitNumToSkip(I, OS);
+      I += 3;
       OS << "// Skip to: " << ((I - Table.begin()) + NumToSkip) << "\n";
       break;
     }
@@ -905,15 +899,8 @@ void DecoderEmitter::emitTable(formatted_raw_ostream &OS, DecoderTable &Table,
       // Fallthrough for OPC_TryDecode.
 
       // 24-bit numtoskip value.
-      uint8_t Byte = *I++;
-      uint32_t NumToSkip = Byte;
-      OS << (unsigned)Byte << ", ";
-      Byte = *I++;
-      OS << (unsigned)Byte << ", ";
-      NumToSkip |= Byte << 8;
-      Byte = *I++;
-      OS << utostr(Byte) << ", ";
-      NumToSkip |= Byte << 16;
+      uint32_t NumToSkip = emitNumToSkip(I, OS);
+      I += 3;
 
       OS << "// Opcode: " << NumberedEncodings[Opc]
          << ", skip to: " << ((I - Table.begin()) + NumToSkip) << "\n";



More information about the llvm-commits mailing list