[WebAssembly][MC] Record limit constraints for table sizes

[wingo]

Summary

This commit adds a full WasmTableType to MCSymbolWasm, differing from
the current situation (just an ElemType) in that it additionally records
a WasmLimits.

We add support for specifying the limits in .S files also, via the
following syntax variations:

.tabletype SYM, ELEMTYPE
.tabletype SYM, ELEMTYPE, MINSIZE
.tabletype SYM, ELEMTYPE, MINSIZE, MAXSIZE

Depends on D99186.

[sbc100]

Great!

llvm/include/llvm/MC/MCSymbolWasm.h

111

Can you use WASM_TYPE_FUNCREF and avoid that cast?

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

860

Using auto all over the place makes this harder to review and goes against llvm policy. I guess its pre-existing code so you don't need to change it here.. but I wish it this had a type name

868

make this into single condition (with no curly braces)?

874

All these casts make me this that WasmTableType should have ValType rather than uint8_t

[wingo]

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

860

Yeah I sympathize. I'll add an explicit type for ElemType here.

874

Yeah. Will need to change in any case once we get typed function references. There are some related cases that should probably change at the same time, like BinaryFormat/Wasm.h:WasmGlobalType::Type.

[wingo]

Address feedback

llvm/include/llvm/MC/MCSymbolWasm.h

class MCSymbolWasm : public MCSymbol {
   bool OmitFromLinkingSection = false;
   mutable bool IsUsedInInitArray = false;
   mutable bool IsUsedInGOT = false;
   Optional<StringRef> ImportModule;
   Optional<StringRef> ImportName;
   Optional<StringRef> ExportName;
   wasm::WasmSignature *Signature = nullptr;
   Optional<wasm::WasmGlobalType> GlobalType;
-  Optional<wasm::ValType> TableType;
+  Optional<wasm::WasmTableType> TableType;
   Optional<wasm::WasmEventType> EventType;
 
   /// An expression describing how to calculate the size of a symbol. If a
   /// symbol has no size this field will be NULL.
   const MCExpr *SymbolSize = nullptr;
 
 public:
   MCSymbolWasm(const StringMapEntry<bool> *Name, bool isTemporary)
public:
   void setImportName(StringRef Name) { ImportName = Name; }
 
   bool hasExportName() const { return ExportName.hasValue(); }
   StringRef getExportName() const { return ExportName.getValue(); }
   void setExportName(StringRef Name) { ExportName = Name; }
 
   bool isFunctionTable() const {
     return isTable() && hasTableType() &&
-           getTableType() == wasm::ValType::FUNCREF;
+           getTableType().ElemType == uint8_t(wasm::ValType::FUNCREF);
   }
   void setFunctionTable() {
     setType(wasm::WASM_SYMBOL_TYPE_TABLE);
     setTableType(wasm::ValType::FUNCREF);
   }
 
   void setUsedInGOT() const { IsUsedInGOT = true; }
   bool isUsedInGOT() const { return IsUsedInGOT; }
 
   void setUsedInInitArray() const { IsUsedInInitArray = true; }
   bool isUsedInInitArray() const { return IsUsedInInitArray; }
 
   const wasm::WasmSignature *getSignature() const { return Signature; }
   void setSignature(wasm::WasmSignature *Sig) { Signature = Sig; }
 
   const wasm::WasmGlobalType &getGlobalType() const {
     assert(GlobalType.hasValue());
     return GlobalType.getValue();
   }
   void setGlobalType(wasm::WasmGlobalType GT) { GlobalType = GT; }
 
   bool hasTableType() const { return TableType.hasValue(); }
-  wasm::ValType getTableType() const {
+  const wasm::WasmTableType &getTableType() const {
     assert(hasTableType());
     return TableType.getValue();
   }
-  void setTableType(wasm::ValType TT) { TableType = TT; }
+  void setTableType(wasm::WasmTableType TT) { TableType = TT; }
+  void setTableType(wasm::ValType VT) {
+    // Declare a table with element type VT and no limits (min size 0, no max
+    // size).
+    wasm::WasmLimits Limits = {wasm::WASM_LIMITS_FLAG_NONE, 0, 0};
+    setTableType({uint8_t(VT), Limits});
+  }
 
   const wasm::WasmEventType &getEventType() const {
     assert(EventType.hasValue());
     return EventType.getValue();
   }
   void setEventType(wasm::WasmEventType ET) { EventType = ET; }
 };
 
 } // end namespace llvm
 
 #endif // LLVM_MC_MCSYMBOLWASM_H

llvm/lib/MC/WasmObjectWriter.cpp

if (!WS.isDefined() && !WS.isComdat()) {
       } else if (WS.isTable()) {
         if (WS.isWeak())
           report_fatal_error("undefined table symbol cannot be weak");
 
         wasm::WasmImport Import;
         Import.Module = WS.getImportModule();
         Import.Field = WS.getImportName();
         Import.Kind = wasm::WASM_EXTERNAL_TABLE;
-        wasm::ValType ElemType = WS.getTableType();
-        Import.Table.ElemType = uint8_t(ElemType);
-        // FIXME: Extend table type to include limits? For now we don't specify
-        // a min or max which does not place any restrictions on the size of the
-        // imported table.
-        Import.Table.Limits = {wasm::WASM_LIMITS_FLAG_NONE, 0, 0};
+        Import.Table = WS.getTableType();
         Imports.push_back(Import);
         assert(WasmIndices.count(&WS) == 0);
         WasmIndices[&WS] = NumTableImports++;
       }
     }
   }
 
   // Add imports for GOT globals
for (const MCSymbol &S : Asm.symbols()) {
           // An import; the index was assigned above
           LLVM_DEBUG(dbgs() << "  -> global index: "
                             << WasmIndices.find(&WS)->second << "\n");
         }
       } else if (WS.isTable()) {
         if (WS.isDefined()) {
           wasm::WasmTable Table;
           Table.Index = NumTableImports + Tables.size();
-          Table.Type.ElemType = static_cast<uint8_t>(WS.getTableType());
-          // FIXME: Work on custom limits is ongoing
-          Table.Type.Limits = {wasm::WASM_LIMITS_FLAG_NONE, 0, 0};
+          Table.Type = WS.getTableType();
           assert(WasmIndices.count(&WS) == 0);
           WasmIndices[&WS] = Table.Index;
           Tables.push_back(Table);
         }
         LLVM_DEBUG(dbgs() << " -> table index: "
                           << WasmIndices.find(&WS)->second << "\n");
       } else if (WS.isEvent()) {
         // C++ exception symbol (__cpp_exception)

llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp

case Symbol:
       break;
     case BrList:
       OS << "BrList:" << BrL.List.size();
       break;
     }
   }
 };
 
+// Perhaps this should go somewhere common.
+static wasm::WasmLimits DefaultLimits() {
+  return {wasm::WASM_LIMITS_FLAG_NONE, 0, 0};
+}
+
 static MCSymbolWasm *GetOrCreateFunctionTableSymbol(MCContext &Ctx,
                                                     const StringRef &Name) {
   MCSymbolWasm *Sym = cast_or_null<MCSymbolWasm>(Ctx.lookupSymbol(Name));
   if (Sym) {
     if (!Sym->isFunctionTable())
       Ctx.reportError(SMLoc(), "symbol is not a wasm funcref table");
   } else {
     Sym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(Name));
#include "WebAssemblyGenAsmMatcher.inc"
 
   void addBlockTypeOperand(OperandVector &Operands, SMLoc NameLoc,
                            WebAssembly::BlockType BT) {
     Operands.push_back(std::make_unique<WebAssemblyOperand>(
         WebAssemblyOperand::Integer, NameLoc, NameLoc,
         WebAssemblyOperand::IntOp{static_cast<int64_t>(BT)}));
   }
 
+  bool parseLimits(wasm::WasmLimits *Limits) {
+    auto Tok = Lexer.getTok();
+    if (!Tok.is(AsmToken::Integer))
+      return error("Expected integer constant, instead got: ", Tok);
+    int64_t Val = Tok.getIntVal();
+    assert(Val >= 0);
+    Limits->Minimum = Val;
+    Parser.Lex();
+
+    if (isNext(AsmToken::Comma)) {
+      Limits->Flags |= wasm::WASM_LIMITS_FLAG_HAS_MAX;
+      auto Tok = Lexer.getTok();
+      if (!Tok.is(AsmToken::Integer))
+        return error("Expected integer constant, instead got: ", Tok);
+      int64_t Val = Tok.getIntVal();
+      assert(Val >= 0);
+      Limits->Maximum = Val;
+      Parser.Lex();
+    }
+    return false;
+  }
+
   bool parseFunctionTableOperand(std::unique_ptr<WebAssemblyOperand> *Op) {
     if (STI->checkFeatures("+reference-types")) {
       // If the reference-types feature is enabled, there is an explicit table
       // operand.  To allow the same assembly to be compiled with or without
       // reference types, we allow the operand to be omitted, in which case we
       // default to __indirect_function_table.
       auto &Tok = Lexer.getTok();
       if (Tok.is(AsmToken::Identifier)) {
if (DirectiveID.getString() == ".globaltype") {
       WasmSym->setGlobalType(
           wasm::WasmGlobalType{uint8_t(Type.getValue()), Mutable});
       // And emit the directive again.
       TOut.emitGlobalType(WasmSym);
       return expect(AsmToken::EndOfStatement, "EOL");
     }
 
     if (DirectiveID.getString() == ".tabletype") {
+      // .tabletype SYM, ELEMTYPE[, MINSIZE[, MAXSIZE]]
       auto SymName = expectIdent();
       if (SymName.empty())
         return true;
       if (expect(AsmToken::Comma, ","))
         return true;
-      auto TypeTok = Lexer.getTok();
-      auto TypeName = expectIdent();
-      if (TypeName.empty())
+
+      auto ElemTypeTok = Lexer.getTok();
+      auto ElemTypeName = expectIdent();
+      if (ElemTypeName.empty())
+        return true;
+      auto ElemType = parseType(ElemTypeName);
+      if (!ElemType)
+        return error("Unknown type in .tabletype directive: ", ElemTypeTok);
+
+      wasm::WasmLimits Limits = DefaultLimits();
+      if (isNext(AsmToken::Comma)) {
+        if (parseLimits(&Limits))
           return true;
-      auto Type = parseType(TypeName);
-      if (!Type)
-        return error("Unknown type in .tabletype directive: ", TypeTok);
+      }
 
       // Now that we have the name and table type, we can actually create the
       // symbol
       auto WasmSym = cast<MCSymbolWasm>(Ctx.getOrCreateSymbol(SymName));
       WasmSym->setType(wasm::WASM_SYMBOL_TYPE_TABLE);
-      WasmSym->setTableType(Type.getValue());
+      wasm::WasmTableType Type = {uint8_t(ElemType.getValue()), Limits};
+      WasmSym->setTableType(Type);
       TOut.emitTableType(WasmSym);
       return expect(AsmToken::EndOfStatement, "EOL");
     }
 
     if (DirectiveID.getString() == ".functype") {
       // This code has to send things to the streamer similar to
       // WebAssemblyAsmPrinter::EmitFunctionBodyStart.
       // TODO: would be good to factor this into a common function, but the

llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp

<< WebAssembly::typeToString(
             static_cast<wasm::ValType>(Sym->getGlobalType().Type));
   if (!Sym->getGlobalType().Mutable)
     OS << ", immutable";
   OS << '\n';
 }
 
 void WebAssemblyTargetAsmStreamer::emitTableType(const MCSymbolWasm *Sym) {
   assert(Sym->isTable());
+  const wasm::WasmTableType &Type = Sym->getTableType();
   OS << "\t.tabletype\t" << Sym->getName() << ", "
-     << WebAssembly::typeToString(Sym->getTableType());
+     << WebAssembly::typeToString(static_cast<wasm::ValType>(Type.ElemType));
+  bool HasMaximum = Type.Limits.Flags & wasm::WASM_LIMITS_FLAG_HAS_MAX;
+  if (Type.Limits.Minimum != 0 || HasMaximum) {
+    OS << ", " << Type.Limits.Minimum;
+    if (HasMaximum)
+      OS << ", " << Type.Limits.Maximum;
+  }
   OS << '\n';
 }
 
 void WebAssemblyTargetAsmStreamer::emitEventType(const MCSymbolWasm *Sym) {
   assert(Sym->isEvent());
   OS << "\t.eventtype\t" << Sym->getName() << " ";
   OS << WebAssembly::typeListToString(Sym->getSignature()->Params);
   OS << "\n";

llvm/test/MC/WebAssembly/tables.s

 
 
 #      CHECK: bar:
 # CHECK-NEXT:         .tabletype bar, funcref
 bar:
     .tabletype bar, funcref
 
 table1:
-    .tabletype table1, funcref
+    .tabletype table1, funcref, 42
 table2:
-    .tabletype table2, funcref
+    .tabletype table2, funcref, 42, 100
 
 # Table instructions
 
 #      CHECK: copy_tables:
 # CHECK-NEXT:         .functype copy_tables (i32, i32) -> ()
 # CHECK-NEXT: local.get 0
 # CHECK-NEXT: local.get 1
 #      CHECK: table.size  table1

 # BIN-NEXT:          Minimum:         0x0
 # BIN-NEXT:      - Index:           1
 # BIN-NEXT:        ElemType:        FUNCREF
 # BIN-NEXT:        Limits:
 # BIN-NEXT:          Minimum:         0x0
 # BIN-NEXT:      - Index:           2
 # BIN-NEXT:        ElemType:        FUNCREF
 # BIN-NEXT:        Limits:
-# BIN-NEXT:          Minimum:         0x0
+# BIN-NEXT:          Minimum:         0x2A
 # BIN-NEXT:      - Index:           3
 # BIN-NEXT:        ElemType:        FUNCREF
 # BIN-NEXT:        Limits:
-# BIN-NEXT:          Minimum:         0x0
+# BIN-NEXT:          Flags:           [ HAS_MAX ]
+# BIN-NEXT:          Minimum:         0x2A
+# BIN-NEXT:          Maximum:         0x64
 
 #      BIN:  - Type:            CODE
 # BIN-NEXT:    Relocations:
 # BIN-NEXT:      - Type:            R_WASM_TABLE_NUMBER_LEB
 # BIN-NEXT:        Index:           2
 # BIN-NEXT:        Offset:          0x9
 # BIN-NEXT:      - Type:            R_WASM_TABLE_NUMBER_LEB
 # BIN-NEXT:        Index:           2