[llvm] [NFC][TableGen] Code cleanup in Wasm disassember emitter (PR #135992)
Rahul Joshi via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 16 10:29:28 PDT 2025
https://github.com/jurahul updated https://github.com/llvm/llvm-project/pull/135992
>From 6ab98dec356e40ccfbd3fc11681273efb8a2a0dd Mon Sep 17 00:00:00 2001
From: Rahul Joshi <rjoshi at nvidia.com>
Date: Wed, 16 Apr 2025 09:46:38 -0700
Subject: [PATCH] [NFC][TableGen] Code cleanup in Wasm disassember emitter
- Use range for loop to iterate over instructions.
- Emit generated code in anonymous namespace instead of `llvm`,
and reduce the scope of this to just the type declarations.
- Emit generated tables as static constexpr
- Replace code to search in operand table with `std::search`.
- Skip the last "null" entry in PrefixTable and use range for
loop to search PrefixTable in the .cpp code.
- Do not generate `WebAssemblyInstructionTableSize` definition as
its already defined in the .cpp file.
- Remove {} for single statement loop/if/else bodies.
---
.../Disassembler/WebAssemblyDisassembler.cpp | 10 +--
.../WebAssemblyDisassemblerEmitter.cpp | 87 +++++++++----------
2 files changed, 46 insertions(+), 51 deletions(-)
diff --git a/llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp b/llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp
index 7bee4af86b52a..0399f9d38e4eb 100644
--- a/llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp
+++ b/llvm/lib/Target/WebAssembly/Disassembler/WebAssemblyDisassembler.cpp
@@ -38,9 +38,9 @@ using DecodeStatus = MCDisassembler::DecodeStatus;
#include "WebAssemblyGenDisassemblerTables.inc"
-namespace {
static constexpr int WebAssemblyInstructionTableSize = 256;
+namespace {
class WebAssemblyDisassembler final : public MCDisassembler {
std::unique_ptr<const MCInstrInfo> MCII;
@@ -171,10 +171,10 @@ MCDisassembler::DecodeStatus WebAssemblyDisassembler::getInstruction(
// If this is a prefix byte, indirect to another table.
if (WasmInst->ET == ET_Prefix) {
WasmInst = nullptr;
- // Linear search, so far only 2 entries.
- for (auto PT = PrefixTable; PT->Table; PT++) {
- if (PT->Prefix == Opc) {
- WasmInst = PT->Table;
+ // Linear search, so far only 4 entries.
+ for (const auto &[Prefix, Table] : PrefixTable) {
+ if (Prefix == Opc) {
+ WasmInst = Table;
break;
}
}
diff --git a/llvm/utils/TableGen/WebAssemblyDisassemblerEmitter.cpp b/llvm/utils/TableGen/WebAssemblyDisassemblerEmitter.cpp
index 5aa573ac857dc..cf82176bfdb99 100644
--- a/llvm/utils/TableGen/WebAssemblyDisassemblerEmitter.cpp
+++ b/llvm/utils/TableGen/WebAssemblyDisassemblerEmitter.cpp
@@ -1,4 +1,4 @@
-//===- WebAssemblyDisassemblerEmitter.cpp - Disassembler tables -*- C++ -*-===//
+//===----------------------------------------------------------------------===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -16,6 +16,7 @@
#include "WebAssemblyDisassemblerEmitter.h"
#include "Common/CodeGenInstruction.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/iterator.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/TableGen/Record.h"
@@ -29,8 +30,8 @@ void llvm::emitWebAssemblyDisassemblerTables(
std::map<unsigned,
std::map<unsigned, std::pair<unsigned, const CodeGenInstruction *>>>
OpcodeTable;
- for (unsigned I = 0; I != NumberedInstructions.size(); ++I) {
- const CodeGenInstruction &CGI = *NumberedInstructions[I];
+ for (const auto &[Idx, CGI] :
+ enumerate(make_pointee_range(NumberedInstructions))) {
const Record &Def = *CGI.TheDef;
if (!Def.getValue("Inst"))
continue;
@@ -75,27 +76,31 @@ void llvm::emitWebAssemblyDisassemblerTables(
}
}
// Set this instruction as the one to use.
- CGIP = {I, &CGI};
+ CGIP = {Idx, &CGI};
}
- OS << "#include \"MCTargetDesc/WebAssemblyMCTargetDesc.h\"\n";
- OS << "\n";
- OS << "namespace llvm {\n\n";
- OS << "static constexpr int WebAssemblyInstructionTableSize = ";
- OS << WebAssemblyInstructionTableSize << ";\n\n";
- OS << "enum EntryType : uint8_t { ";
- OS << "ET_Unused, ET_Prefix, ET_Instruction };\n\n";
- OS << "struct WebAssemblyInstruction {\n";
- OS << " uint16_t Opcode;\n";
- OS << " EntryType ET;\n";
- OS << " uint8_t NumOperands;\n";
- OS << " uint16_t OperandStart;\n";
- OS << "};\n\n";
+
+ OS << R"(
+#include "MCTargetDesc/WebAssemblyMCTargetDesc.h"
+
+namespace {
+enum EntryType : uint8_t { ET_Unused, ET_Prefix, ET_Instruction };
+
+struct WebAssemblyInstruction {
+ uint16_t Opcode;
+ EntryType ET;
+ uint8_t NumOperands;
+ uint16_t OperandStart;
+};
+} // end anonymous namespace
+
+)";
+
std::vector<std::string> OperandTable, CurOperandList;
// Output one table per prefix.
for (const auto &[Prefix, Table] : OpcodeTable) {
if (Table.empty())
continue;
- OS << "WebAssemblyInstruction InstructionTable" << Prefix;
+ OS << "static constexpr WebAssemblyInstruction InstructionTable" << Prefix;
OS << "[] = {\n";
for (unsigned I = 0; I < WebAssemblyInstructionTableSize; I++) {
auto InstIt = Table.find(I);
@@ -116,53 +121,43 @@ void llvm::emitWebAssemblyDisassemblerTables(
}
// See if we already have stored this sequence before. This is not
// strictly necessary but makes the table really small.
- size_t OperandStart = OperandTable.size();
- if (CurOperandList.size() <= OperandTable.size()) {
- for (size_t J = 0; J <= OperandTable.size() - CurOperandList.size();
- ++J) {
- size_t K = 0;
- for (; K < CurOperandList.size(); ++K) {
- if (OperandTable[J + K] != CurOperandList[K])
- break;
- }
- if (K == CurOperandList.size()) {
- OperandStart = J;
- break;
- }
- }
- }
+ auto SearchI =
+ std::search(OperandTable.begin(), OperandTable.end(),
+ CurOperandList.begin(), CurOperandList.end());
+ OS << std::distance(OperandTable.begin(), SearchI);
// Store operands if no prior occurrence.
- if (OperandStart == OperandTable.size()) {
+ if (SearchI == OperandTable.end())
llvm::append_range(OperandTable, CurOperandList);
- }
- OS << OperandStart;
} else {
auto PrefixIt = OpcodeTable.find(I);
// If we have a non-empty table for it that's not 0, this is a prefix.
- if (PrefixIt != OpcodeTable.end() && I && !Prefix) {
+ if (PrefixIt != OpcodeTable.end() && I && !Prefix)
OS << " { 0, ET_Prefix, 0, 0";
- } else {
+ else
OS << " { 0, ET_Unused, 0, 0";
- }
}
OS << " },\n";
}
OS << "};\n\n";
}
// Create a table of all operands:
- OS << "const uint8_t OperandTable[] = {\n";
- for (auto &Op : OperandTable) {
+ OS << "static constexpr uint8_t OperandTable[] = {\n";
+ for (const auto &Op : OperandTable)
OS << " " << Op << ",\n";
- }
OS << "};\n\n";
+
// Create a table of all extension tables:
- OS << "struct { uint8_t Prefix; const WebAssemblyInstruction *Table; }\n";
- OS << "PrefixTable[] = {\n";
+ OS << R"(
+static constexpr struct {
+ uint8_t Prefix;
+ const WebAssemblyInstruction *Table;
+} PrefixTable[] = {
+)";
+
for (const auto &[Prefix, Table] : OpcodeTable) {
if (Table.empty() || !Prefix)
continue;
OS << " { " << Prefix << ", InstructionTable" << Prefix << " },\n";
}
- OS << " { 0, nullptr }\n};\n\n";
- OS << "} // end namespace llvm\n";
+ OS << "};\n";
}
More information about the llvm-commits
mailing list