[WebAssembly][NFC] Remove WebAssemblyStackifier TableGen backend

[tlively]

Summary

Replace its functionality with a TableGen InstrInfo relational
instruction mapping. Although arguably more complex than the TableGen
backend, the relational mapping is a smaller maintenance burden than a
TableGen backend.

[tlively]

• Add BaseName to BR_TABLE instructions and add assert

[aardappel]

Hah, generally looks even neater :)

lib/Target/WebAssembly/WebAssemblyInstrFormats.td

21

Why does this need to be a string instead of a bit?

[aheejin]

lib/Transforms/IPO/HotColdSplitting.cpp

153

I think this change has been committed, so you can remove this change

[aheejin]

This looks neat!

[dschuff]

lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp

36

Probably should #undef GET_INSTRMAP_INFO afterwards just for completeness.

[tlively]

• Rebase

[tlively]

lib/Target/WebAssembly/WebAssemblyInstrFormats.td

21

I specified that the values of the StackBased form the columns of the relational mapping. Internally, TableGen will uses these values to form identifiers in an enum. I guess since the values are used as part of identifiers, TableGen requires them to be strings. It seemed better to make this field a string than to introduce a redundant string field that contains the same information.

lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp

36

This is done automatically inside WebAssemblyGenInstrInfo.inc. Other includes of TableGen files do not have an undef either.

lib/Transforms/IPO/HotColdSplitting.cpp

153

Oops! I didn't mean to get this in here.

[aheejin]

Thanks!

utils/TableGen/WebAssemblyDisassemblerEmitter.cpp

51

This looks a bit arcane.. but it's pre-existing, so fine I guess.

[aheejin]

Oh, and maybe it's better to mark this as NFC.

[aardappel]

lib/Target/WebAssembly/WebAssemblyInstrFormats.td

21

Ah ok, thanks.

lib/Target/WebAssembly/CMakeLists.txt

 set(LLVM_TARGET_DEFINITIONS WebAssembly.td)
 
 tablegen(LLVM WebAssemblyGenAsmMatcher.inc -gen-asm-matcher)
 tablegen(LLVM WebAssemblyGenAsmWriter.inc -gen-asm-writer)
 tablegen(LLVM WebAssemblyGenDAGISel.inc -gen-dag-isel)
 tablegen(LLVM WebAssemblyGenDisassemblerTables.inc -gen-disassembler)
 tablegen(LLVM WebAssemblyGenFastISel.inc -gen-fast-isel)
 tablegen(LLVM WebAssemblyGenInstrInfo.inc -gen-instr-info)
 tablegen(LLVM WebAssemblyGenMCCodeEmitter.inc -gen-emitter)
 tablegen(LLVM WebAssemblyGenRegisterInfo.inc -gen-register-info)
 tablegen(LLVM WebAssemblyGenSubtargetInfo.inc -gen-subtarget)
-tablegen(LLVM WebAssemblyGenStackifier.inc -gen-wasm-stackifier)
 
 add_public_tablegen_target(WebAssemblyCommonTableGen)
 
 add_llvm_target(WebAssemblyCodeGen
   WebAssemblyAddMissingPrototypes.cpp
   WebAssemblyArgumentMove.cpp
   WebAssemblyAsmPrinter.cpp
   WebAssemblyCallIndirectFixup.cpp

lib/Target/WebAssembly/WebAssemblyInstrControl.td

 // currently.
 // Set TSFlags{0} to 1 to indicate that the variable_ops are immediates.
 // Set TSFlags{1} to 1 to indicate that the immediates represent labels.
 // FIXME: this can't inherit from I<> since there is no way to inherit from a
 // multiclass and still have the let statements.
 let isTerminator = 1, hasCtrlDep = 1, isBarrier = 1 in {
 let isCodeGenOnly = 1 in
 def BR_TABLE_I32 : NI<(outs), (ins I32:$index, variable_ops),
-                      [(WebAssemblybr_table I32:$index)], 0,
+                      [(WebAssemblybr_table I32:$index)], "false",
                       "br_table \t$index", 0x0e> {
   let TSFlags{0} = 1;
   let TSFlags{1} = 1;
 }
+let BaseName = "BR_TABLE_I32" in
 def BR_TABLE_I32_S : NI<(outs), (ins variable_ops),
-                        [], 1,
+                        [], "true",
                         "br_table", 0x0e> {
   let TSFlags{0} = 1;
   let TSFlags{1} = 1;
 }
 let isCodeGenOnly = 1 in
 def BR_TABLE_I64 : NI<(outs), (ins I64:$index, variable_ops),
-                      [(WebAssemblybr_table I64:$index)], 0,
+                      [(WebAssemblybr_table I64:$index)], "false",
                       "br_table \t$index"> {
   let TSFlags{0} = 1;
   let TSFlags{1} = 1;
 }
+let BaseName = "BR_TABLE_I64" in
 def BR_TABLE_I64_S : NI<(outs), (ins variable_ops),
-                        [], 1,
+                        [], "true",
                         "br_table"> {
   let TSFlags{0} = 1;
   let TSFlags{1} = 1;
 }
 } // isTerminator = 1, hasCtrlDep = 1, isBarrier = 1
 
 // This is technically a control-flow instruction, since all it affects is the
 // IP.

lib/Target/WebAssembly/WebAssemblyInstrFormats.td

 /// \file
 /// WebAssembly instruction format definitions.
 ///
 //===----------------------------------------------------------------------===//
 
 // WebAssembly Instruction Format.
 // We instantiate 2 of these for every actual instruction (register based
 // and stack based), see below.
-class WebAssemblyInst<bits<32> inst, string asmstr, bit stack> : Instruction {
-  field bits<32> Inst = inst; // Instruction encoding.
-  field bit StackBased = stack;
+class WebAssemblyInst<bits<32> inst, string asmstr, string stack> : StackRel,
+  Instruction {
+  bits<32> Inst = inst; // Instruction encoding.
+  string StackBased = stack;
+  string BaseName = NAME;
   let Namespace   = "WebAssembly";
   let Pattern     = [];
   let AsmString   = asmstr;
 }
 
 // Normal instructions. Default instantiation of a WebAssemblyInst.
-class NI<dag oops, dag iops, list<dag> pattern, bit stack, string asmstr = "",
-         bits<32> inst = -1>
+class NI<dag oops, dag iops, list<dag> pattern, string stack,
+         string asmstr = "", bits<32> inst = -1>
     : WebAssemblyInst<inst, asmstr, stack> {
   dag OutOperandList = oops;
   dag InOperandList  = iops;
   let Pattern        = pattern;
   let Defs           = [ARGUMENTS];
 }
 
 // Generates both register and stack based versions of one actual instruction.
 // We have 2 sets of operands (oops & iops) for the register and stack
 // based version of this instruction, as well as the corresponding asmstr.
 // The register versions have virtual-register operands which correspond to wasm
 // locals or stack locations. Each use and def of the register corresponds to an
 // implicit get_local / set_local or access of stack operands in wasm. These
 // instructions are used for ISel and all MI passes. The stack versions of the
 // instructions do not have register operands (they implicitly operate on the
 // stack), and get_locals and set_locals are explicit. The register instructions
 // are converted to their corresponding stack instructions before lowering to
 // MC.
 // Every instruction should want to be based on this multi-class to guarantee
 // there is always an equivalent pair of instructions.
 multiclass I<dag oops_r, dag iops_r, dag oops_s, dag iops_s,
              list<dag> pattern_r, string asmstr_r = "", string asmstr_s = "",
              bits<32> inst = -1> {
   let isCodeGenOnly = 1 in
-  def "" : NI<oops_r, iops_r, pattern_r, 0, asmstr_r, inst>;
-  def _S : NI<oops_s, iops_s, [], 1, asmstr_s, inst>;
+  def "" : NI<oops_r, iops_r, pattern_r, "false", asmstr_r, inst>;
+  let BaseName = NAME in
+  def _S : NI<oops_s, iops_s, [], "true", asmstr_s, inst>;
 }
 
 // For instructions that have no register ops, so both sets are the same.
 multiclass NRI<dag oops, dag iops, list<dag> pattern, string asmstr = "",
                bits<32> inst = -1> {
   defm "": I<oops, iops, oops, iops, pattern, asmstr, asmstr, inst>;
 }

lib/Target/WebAssembly/WebAssemblyInstrInfo.td

 } // OperandType = "OPERAND_SIGNATURE"
 
 let OperandType = "OPERAND_TYPEINDEX" in
 def TypeIndex : Operand<i32>;
 
 } // OperandNamespace = "WebAssembly"
 
 //===----------------------------------------------------------------------===//
+// WebAssembly Register to Stack instruction mapping
+//===----------------------------------------------------------------------===//
+
+class StackRel;
+def getStackOpcode : InstrMapping {
+  let FilterClass = "StackRel";
+  let RowFields = ["BaseName"];
+  let ColFields = ["StackBased"];
+  let KeyCol = ["false"];
+  let ValueCols = [["true"]];
+}
+
+//===----------------------------------------------------------------------===//
 // WebAssembly Instruction Format Definitions.
 //===----------------------------------------------------------------------===//
 
 include "WebAssemblyInstrFormats.td"
 
 //===----------------------------------------------------------------------===//
 // Additional instructions.
 //===----------------------------------------------------------------------===//

lib/Target/WebAssembly/WebAssemblyMCInstLower.cpp

 #include "llvm/MC/MCContext.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCSymbolWasm.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 
+// Defines llvm::WebAssembly::getStackOpcode to convert register instructions to
+// stack instructions
+#define GET_INSTRMAP_INFO 1
+#include "WebAssemblyGenInstrInfo.inc"
+
 // This disables the removal of registers when lowering into MC, as required
 // by some current tests.
 static cl::opt<bool>
     WasmKeepRegisters("wasm-keep-registers", cl::Hidden,
                       cl::desc("WebAssembly: output stack registers in"
                                " instruction output for test purposes only."),
                       cl::init(false));
 
-static unsigned regInstructionToStackInstruction(unsigned OpCode);
 static void removeRegisterOperands(const MachineInstr *MI, MCInst &OutMI);
 
 MCSymbol *
 WebAssemblyMCInstLower::GetGlobalAddressSymbol(const MachineOperand &MO) const {
   const GlobalValue *Global = MO.getGlobal();
   MCSymbolWasm *WasmSym = cast<MCSymbolWasm>(Printer.getSymbol(Global));
 
   if (const auto *FuncTy = dyn_cast<FunctionType>(Global->getValueType())) {
static void removeRegisterOperands(const MachineInstr *MI, MCInst &OutMI) {
   // until this transition point.
   // FIXME: we are not processing inline assembly, which contains register
   // operands, because it is used by later target generic code.
   if (MI->isDebugInstr() || MI->isLabel() || MI->isInlineAsm())
     return;
 
   // Transform to _S instruction.
   auto RegOpcode = OutMI.getOpcode();
-  auto StackOpcode = regInstructionToStackInstruction(RegOpcode);
+  auto StackOpcode = WebAssembly::getStackOpcode(RegOpcode);
+  assert(StackOpcode != -1 && "Failed to stackify instruction");
   OutMI.setOpcode(StackOpcode);
 
   // Remove register operands.
   for (auto I = OutMI.getNumOperands(); I; --I) {
     auto &MO = OutMI.getOperand(I - 1);
     if (MO.isReg()) {
       OutMI.erase(&MO);
     }
   }
 }
-
-static unsigned regInstructionToStackInstruction(unsigned OpCode) {
-  // For most opcodes, this function could have been implemented as "return
-  // OpCode + 1", but since table-gen alphabetically sorts them, this cannot be
-  // guaranteed (see e.g. BR and BR_IF). Instead we use a giant switch statement
-  // generated by a custom TableGen backend (WebAssemblyStackifierEmitter.cpp)
-  // that emits switch cases of the form
-  //
-  //   case WebAssembly::RegisterInstr: return WebAssembly::StackInstr;
-  //
-  // for every pair of equivalent register and stack instructions.
-  switch (OpCode) {
-  default:
-    llvm_unreachable(
-        "unknown WebAssembly instruction in WebAssemblyMCInstLower pass");
-#include "WebAssemblyGenStackifier.inc"
-  }
-}

lib/Transforms/IPO/HotColdSplitting.cpp

if (const CallInst *CI = dyn_cast<CallInst>(&I)) {
       // Assume that inline assembly is hot code.
       if (isa<InlineAsm>(CI->getCalledValue()))
         return false;
     }
   return false;
 }
 
 static bool returnsOrHasSideEffects(const BasicBlock &BB) {
-  const TerminatorInst *I = BB.getTerminator();
+  const Instruction *I = BB.getTerminator();
   if (isa<ReturnInst>(I) || isa<IndirectBrInst>(I) || isa<InvokeInst>(I))
     return true;
 
   for (const Instruction &I : BB)
     if (const CallInst *CI = dyn_cast<CallInst>(&I)) {
       if (CI->hasFnAttr(Attribute::NoReturn))
         return true;
 

utils/TableGen/CMakeLists.txt

add_tablegen(llvm-tblgen LLVM
   TableGen.cpp
   Types.cpp
   X86DisassemblerTables.cpp
   X86EVEX2VEXTablesEmitter.cpp
   X86FoldTablesEmitter.cpp
   X86ModRMFilters.cpp
   X86RecognizableInstr.cpp
   WebAssemblyDisassemblerEmitter.cpp
-  WebAssemblyStackifierEmitter.cpp
   CTagsEmitter.cpp
   )
 set_target_properties(llvm-tblgen PROPERTIES FOLDER "Tablegenning")

utils/TableGen/TableGen.cpp

enum ActionType {
   GenOptParserDefs,
   GenCTags,
   GenAttributes,
   GenSearchableTables,
   GenGlobalISel,
   GenX86EVEX2VEXTables,
   GenX86FoldTables,
   GenRegisterBank,
-  GenWebAssemblyStackifier,
 };
 
 namespace {
   cl::opt<ActionType>
   Action(cl::desc("Action to perform:"),
          cl::values(clEnumValN(PrintRecords, "print-records",
                                "Print all records to stdout (default)"),
                     clEnumValN(DumpJSON, "dump-json",
Action(cl::desc("Action to perform:"),
                                "Generate generic binary-searchable table"),
                     clEnumValN(GenGlobalISel, "gen-global-isel",
                                "Generate GlobalISel selector"),
                     clEnumValN(GenX86EVEX2VEXTables, "gen-x86-EVEX2VEX-tables",
                                "Generate X86 EVEX to VEX compress tables"),
                     clEnumValN(GenX86FoldTables, "gen-x86-fold-tables",
                                "Generate X86 fold tables"),
                     clEnumValN(GenRegisterBank, "gen-register-bank",
-                               "Generate registers bank descriptions"),
-                    clEnumValN(GenWebAssemblyStackifier, "gen-wasm-stackifier",
-                               "Generate WebAssembly stackification cases")));
+                               "Generate registers bank descriptions")));
 
   cl::OptionCategory PrintEnumsCat("Options for -print-enums");
   cl::opt<std::string>
   Class("class", cl::desc("Print Enum list for this class"),
         cl::value_desc("class name"), cl::cat(PrintEnumsCat));
 
 bool LLVMTableGenMain(raw_ostream &OS, RecordKeeper &Records) {
   switch (Action) {
case GenRegisterBank:
     EmitRegisterBank(Records, OS);
     break;
   case GenX86EVEX2VEXTables:
     EmitX86EVEX2VEXTables(Records, OS);
     break;
   case GenX86FoldTables:
     EmitX86FoldTables(Records, OS);
     break;
-  case GenWebAssemblyStackifier:
-    EmitWebAssemblyStackifier(Records, OS);
-    break;
   }
 
   return false;
 }
 }
 
 int main(int argc, char **argv) {
   sys::PrintStackTraceOnErrorSignal(argv[0]);

utils/TableGen/TableGenBackends.h

 void EmitOptParser(RecordKeeper &RK, raw_ostream &OS);
 void EmitCTags(RecordKeeper &RK, raw_ostream &OS);
 void EmitAttributes(RecordKeeper &RK, raw_ostream &OS);
 void EmitSearchableTables(RecordKeeper &RK, raw_ostream &OS);
 void EmitGlobalISel(RecordKeeper &RK, raw_ostream &OS);
 void EmitX86EVEX2VEXTables(RecordKeeper &RK, raw_ostream &OS);
 void EmitX86FoldTables(RecordKeeper &RK, raw_ostream &OS);
 void EmitRegisterBank(RecordKeeper &RK, raw_ostream &OS);
-void EmitWebAssemblyStackifier(RecordKeeper &RK, raw_ostream &OS);
 
 } // End llvm namespace
 
 #endif

utils/TableGen/WebAssemblyDisassemblerEmitter.cpp

auto Opc = static_cast<unsigned>(
         reinterpret_cast<IntInit *>(Inst.convertInitializerTo(IntRecTy::get()))
             ->getValue());
     if (Opc == 0xFFFFFFFF)
       continue; // No opcode defined.
     assert(Opc <= 0xFFFF);
     auto Prefix = Opc >> 8;
     Opc = Opc & 0xFF;
     auto &CGIP = OpcodeTable[Prefix][Opc];
-    // All wasm instructions have a StackBased fieldof type bit, we only want
-    // the instructions for which this is 1.
-    auto Bit = Def.getValue("StackBased")->getValue()->
-                 getCastTo(BitRecTy::get());
-    auto IsStackBased = Bit && reinterpret_cast<const BitInit *>(Bit)
-                                 ->getValue();
+    // All wasm instructions have a StackBased field of type string, we only
+    // want the instructions for which this is "true".
+    auto StackString =
+        Def.getValue("StackBased")->getValue()->getCastTo(StringRecTy::get());
+    auto IsStackBased =
+        StackString &&
+        reinterpret_cast<const StringInit *>(StackString)->getValue() == "true";
     if (IsStackBased && !CGIP.second) {
       // this picks the first of many typed variants, which is
       // currently the except_ref one, though this shouldn't matter for
       // disassembly purposes.
       CGIP = std::make_pair(I, &CGI);
     }
   }
   OS << "#include \"MCTargetDesc/WebAssemblyMCTargetDesc.h\"\n";

utils/TableGen/WebAssemblyStackifierEmitter.cpp

-//===- WebAssemblyStackifierEmitter.cpp - Stackifier cases ------*- C++ -*-===//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// This file emits the switch statement cases to translate WebAssembly
-// instructions to their stack forms.
-//
-//===----------------------------------------------------------------------===//
-
-#include "WebAssemblyDisassemblerEmitter.h"
-#include "llvm/TableGen/Record.h"
-
-namespace llvm {
-
-// Find all register WebAssembly instructions and their corresponding stack
-// instructions. For each pair, emit a switch case of the form
-//
-//   case WebAssembly::RegisterInstr: return WebAssembly::StackInstr;
-//
-// For example,
-//
-//   case WebAssembly::ADD_I32: return WebAssembly::ADD_I32_S;
-//
-// This is useful for converting instructions from their register form to their
-// equivalent stack form.
-void EmitWebAssemblyStackifier(RecordKeeper &RK, raw_ostream &OS) {
-  Record *InstrClass = RK.getClass("WebAssemblyInst");
-  for (auto &RecordPair : RK.getDefs()) {
-    if (!RecordPair.second->isSubClassOf(InstrClass))
-      continue;
-    bool IsStackBased = RecordPair.second->getValueAsBit("StackBased");
-    if (IsStackBased)
-      continue;
-    OS << "  case WebAssembly::" << RecordPair.first << ": return "
-       << "WebAssembly::" << RecordPair.first << "_S;\n";
-  }
-}
-
-} // namespace llvm