Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WebAssembly] Use ValType instead of integer types to model wasm tables #78012

Merged
merged 2 commits into from
Jan 17, 2024

Conversation

dschuff
Copy link
Member

@dschuff dschuff commented Jan 13, 2024

LLVM models some features found in the binary format with raw integers and others with nested or enumerated types. This PR switches modeling of tables and segments to use wasm::ValType rather than uint32_t. This NFC change is in preparation for modeling more reference types, but IMO is also cleaner and closer to the spec.

LLVM models some features found in the binary format with raw integers and
others with nested or enumerated types. This PR switches modeling of tables
and segments to use wasm::ValType rather than uint32_t. This NFC change is in
preparation for modeling more reference types, but IMO is also cleaner and
closer to the spec.
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 13, 2024

@llvm/pr-subscribers-lld-wasm
@llvm/pr-subscribers-objectyaml

@llvm/pr-subscribers-backend-webassembly

Author: Derek Schuff (dschuff)

Changes

LLVM models some features found in the binary format with raw integers and others with nested or enumerated types. This PR switches modeling of tables and segments to use wasm::ValType rather than uint32_t. This NFC change is in preparation for modeling more reference types, but IMO is also cleaner and closer to the spec.


Patch is 22.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78012.diff

10 Files Affected:

  • (modified) lld/wasm/InputFiles.cpp (+1-1)
  • (modified) lld/wasm/SymbolTable.cpp (+2-2)
  • (modified) llvm/include/llvm/BinaryFormat/Wasm.h (+200-198)
  • (modified) llvm/include/llvm/BinaryFormat/WasmTraits.h (+2-2)
  • (modified) llvm/include/llvm/MC/MCSymbolWasm.h (+2-2)
  • (modified) llvm/lib/MC/WasmObjectWriter.cpp (+1-1)
  • (modified) llvm/lib/Object/WasmObjectFile.cpp (+12-11)
  • (modified) llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp (+1-1)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp (+1-1)
  • (modified) llvm/tools/obj2yaml/wasm2yaml.cpp (+2-2)
diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp
index 5709a5ced584c72..97c88587231ba93 100644
--- a/lld/wasm/InputFiles.cpp
+++ b/lld/wasm/InputFiles.cpp
@@ -320,7 +320,7 @@ void ObjFile::addLegacyIndirectFunctionTableIfNeeded(
   // it has an unexpected name or type, assume that it's not actually the
   // indirect function table.
   if (tableImport->Field != functionTableName ||
-      tableImport->Table.ElemType != uint8_t(ValType::FUNCREF)) {
+      tableImport->Table.ElemType != ValType::FUNCREF) {
     error(toString(this) + ": table import " + Twine(tableImport->Field) +
           " is missing a symbol table entry.");
     return;
diff --git a/lld/wasm/SymbolTable.cpp b/lld/wasm/SymbolTable.cpp
index 76370525c371995..b5c138cd76392d2 100644
--- a/lld/wasm/SymbolTable.cpp
+++ b/lld/wasm/SymbolTable.cpp
@@ -676,7 +676,7 @@ Symbol *SymbolTable::addUndefinedTag(StringRef name,
 TableSymbol *SymbolTable::createUndefinedIndirectFunctionTable(StringRef name) {
   WasmLimits limits{0, 0, 0}; // Set by the writer.
   WasmTableType *type = make<WasmTableType>();
-  type->ElemType = uint8_t(ValType::FUNCREF);
+  type->ElemType = ValType::FUNCREF;
   type->Limits = limits;
   StringRef module(defaultModule);
   uint32_t flags = config->exportTable ? 0 : WASM_SYMBOL_VISIBILITY_HIDDEN;
@@ -690,7 +690,7 @@ TableSymbol *SymbolTable::createUndefinedIndirectFunctionTable(StringRef name) {
 TableSymbol *SymbolTable::createDefinedIndirectFunctionTable(StringRef name) {
   const uint32_t invalidIndex = -1;
   WasmLimits limits{0, 0, 0}; // Set by the writer.
-  WasmTableType type{uint8_t(ValType::FUNCREF), limits};
+  WasmTableType type{ValType::FUNCREF, limits};
   WasmTable desc{invalidIndex, type, name};
   InputTable *table = make<InputTable>(desc, nullptr);
   uint32_t flags = config->exportTable ? 0 : WASM_SYMBOL_VISIBILITY_HIDDEN;
diff --git a/llvm/include/llvm/BinaryFormat/Wasm.h b/llvm/include/llvm/BinaryFormat/Wasm.h
index c7658cc7b7432b3..9921af5e52a7338 100644
--- a/llvm/include/llvm/BinaryFormat/Wasm.h
+++ b/llvm/include/llvm/BinaryFormat/Wasm.h
@@ -31,11 +31,207 @@ const uint32_t WasmMetadataVersion = 0x2;
 // Wasm uses a 64k page size
 const uint32_t WasmPageSize = 65536;
 
+enum : unsigned {
+  WASM_SEC_CUSTOM = 0,     // Custom / User-defined section
+  WASM_SEC_TYPE = 1,       // Function signature declarations
+  WASM_SEC_IMPORT = 2,     // Import declarations
+  WASM_SEC_FUNCTION = 3,   // Function declarations
+  WASM_SEC_TABLE = 4,      // Indirect function table and other tables
+  WASM_SEC_MEMORY = 5,     // Memory attributes
+  WASM_SEC_GLOBAL = 6,     // Global declarations
+  WASM_SEC_EXPORT = 7,     // Exports
+  WASM_SEC_START = 8,      // Start function declaration
+  WASM_SEC_ELEM = 9,       // Elements section
+  WASM_SEC_CODE = 10,      // Function bodies (code)
+  WASM_SEC_DATA = 11,      // Data segments
+  WASM_SEC_DATACOUNT = 12, // Data segment count
+  WASM_SEC_TAG = 13,       // Tag declarations
+  WASM_SEC_LAST_KNOWN = WASM_SEC_TAG,
+};
+
+// Type immediate encodings used in various contexts.
+enum : unsigned {
+  WASM_TYPE_I32 = 0x7F,
+  WASM_TYPE_I64 = 0x7E,
+  WASM_TYPE_F32 = 0x7D,
+  WASM_TYPE_F64 = 0x7C,
+  WASM_TYPE_V128 = 0x7B,
+  WASM_TYPE_FUNCREF = 0x70,
+  WASM_TYPE_EXTERNREF = 0x6F,
+  WASM_TYPE_FUNC = 0x60,
+  WASM_TYPE_NORESULT = 0x40, // for blocks with no result values
+};
+
+// Kinds of externals (for imports and exports).
+enum : unsigned {
+  WASM_EXTERNAL_FUNCTION = 0x0,
+  WASM_EXTERNAL_TABLE = 0x1,
+  WASM_EXTERNAL_MEMORY = 0x2,
+  WASM_EXTERNAL_GLOBAL = 0x3,
+  WASM_EXTERNAL_TAG = 0x4,
+};
+
+// Opcodes used in initializer expressions.
+enum : unsigned {
+  WASM_OPCODE_END = 0x0b,
+  WASM_OPCODE_CALL = 0x10,
+  WASM_OPCODE_LOCAL_GET = 0x20,
+  WASM_OPCODE_LOCAL_SET = 0x21,
+  WASM_OPCODE_LOCAL_TEE = 0x22,
+  WASM_OPCODE_GLOBAL_GET = 0x23,
+  WASM_OPCODE_GLOBAL_SET = 0x24,
+  WASM_OPCODE_I32_STORE = 0x36,
+  WASM_OPCODE_I64_STORE = 0x37,
+  WASM_OPCODE_I32_CONST = 0x41,
+  WASM_OPCODE_I64_CONST = 0x42,
+  WASM_OPCODE_F32_CONST = 0x43,
+  WASM_OPCODE_F64_CONST = 0x44,
+  WASM_OPCODE_I32_ADD = 0x6a,
+  WASM_OPCODE_I32_SUB = 0x6b,
+  WASM_OPCODE_I32_MUL = 0x6c,
+  WASM_OPCODE_I64_ADD = 0x7c,
+  WASM_OPCODE_I64_SUB = 0x7d,
+  WASM_OPCODE_I64_MUL = 0x7e,
+  WASM_OPCODE_REF_NULL = 0xd0,
+};
+
+
+// Opcodes used in synthetic functions.
+enum : unsigned {
+  WASM_OPCODE_BLOCK = 0x02,
+  WASM_OPCODE_BR = 0x0c,
+  WASM_OPCODE_BR_TABLE = 0x0e,
+  WASM_OPCODE_RETURN = 0x0f,
+  WASM_OPCODE_DROP = 0x1a,
+  WASM_OPCODE_MISC_PREFIX = 0xfc,
+  WASM_OPCODE_MEMORY_INIT = 0x08,
+  WASM_OPCODE_MEMORY_FILL = 0x0b,
+  WASM_OPCODE_DATA_DROP = 0x09,
+  WASM_OPCODE_ATOMICS_PREFIX = 0xfe,
+  WASM_OPCODE_ATOMIC_NOTIFY = 0x00,
+  WASM_OPCODE_I32_ATOMIC_WAIT = 0x01,
+  WASM_OPCODE_I32_ATOMIC_STORE = 0x17,
+  WASM_OPCODE_I32_RMW_CMPXCHG = 0x48,
+};
+
+enum : unsigned {
+  WASM_LIMITS_FLAG_NONE = 0x0,
+  WASM_LIMITS_FLAG_HAS_MAX = 0x1,
+  WASM_LIMITS_FLAG_IS_SHARED = 0x2,
+  WASM_LIMITS_FLAG_IS_64 = 0x4,
+};
+
+enum : unsigned {
+  WASM_DATA_SEGMENT_IS_PASSIVE = 0x01,
+  WASM_DATA_SEGMENT_HAS_MEMINDEX = 0x02,
+};
+
+enum : unsigned {
+  WASM_ELEM_SEGMENT_IS_PASSIVE = 0x01,
+  WASM_ELEM_SEGMENT_HAS_TABLE_NUMBER = 0x02,
+  WASM_ELEM_SEGMENT_HAS_INIT_EXPRS = 0x04,
+};
+const unsigned WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND = 0x3;
+
+// Feature policy prefixes used in the custom "target_features" section
+enum : uint8_t {
+  WASM_FEATURE_PREFIX_USED = '+',
+  WASM_FEATURE_PREFIX_REQUIRED = '=',
+  WASM_FEATURE_PREFIX_DISALLOWED = '-',
+};
+
+// Kind codes used in the custom "name" section
+enum : unsigned {
+  WASM_NAMES_MODULE = 0,
+  WASM_NAMES_FUNCTION = 1,
+  WASM_NAMES_LOCAL = 2,
+  WASM_NAMES_GLOBAL = 7,
+  WASM_NAMES_DATA_SEGMENT = 9,
+};
+
+// Kind codes used in the custom "linking" section
+enum : unsigned {
+  WASM_SEGMENT_INFO = 0x5,
+  WASM_INIT_FUNCS = 0x6,
+  WASM_COMDAT_INFO = 0x7,
+  WASM_SYMBOL_TABLE = 0x8,
+};
+
+// Kind codes used in the custom "dylink" section
+enum : unsigned {
+  WASM_DYLINK_MEM_INFO = 0x1,
+  WASM_DYLINK_NEEDED = 0x2,
+  WASM_DYLINK_EXPORT_INFO = 0x3,
+  WASM_DYLINK_IMPORT_INFO = 0x4,
+};
+
+// Kind codes used in the custom "linking" section in the WASM_COMDAT_INFO
+enum : unsigned {
+  WASM_COMDAT_DATA = 0x0,
+  WASM_COMDAT_FUNCTION = 0x1,
+  // GLOBAL, TAG, and TABLE are in here but LLVM doesn't use them yet.
+  WASM_COMDAT_SECTION = 0x5,
+};
+
+// Kind codes used in the custom "linking" section in the WASM_SYMBOL_TABLE
+enum WasmSymbolType : unsigned {
+  WASM_SYMBOL_TYPE_FUNCTION = 0x0,
+  WASM_SYMBOL_TYPE_DATA = 0x1,
+  WASM_SYMBOL_TYPE_GLOBAL = 0x2,
+  WASM_SYMBOL_TYPE_SECTION = 0x3,
+  WASM_SYMBOL_TYPE_TAG = 0x4,
+  WASM_SYMBOL_TYPE_TABLE = 0x5,
+};
+
+enum WasmSegmentFlag : unsigned {
+  WASM_SEG_FLAG_STRINGS = 0x1,
+  WASM_SEG_FLAG_TLS = 0x2,
+};
+
+// Kinds of tag attributes.
+enum WasmTagAttribute : uint8_t {
+  WASM_TAG_ATTRIBUTE_EXCEPTION = 0x0,
+};
+
+const unsigned WASM_SYMBOL_BINDING_MASK = 0x3;
+const unsigned WASM_SYMBOL_VISIBILITY_MASK = 0xc;
+
+const unsigned WASM_SYMBOL_BINDING_GLOBAL = 0x0;
+const unsigned WASM_SYMBOL_BINDING_WEAK = 0x1;
+const unsigned WASM_SYMBOL_BINDING_LOCAL = 0x2;
+const unsigned WASM_SYMBOL_VISIBILITY_DEFAULT = 0x0;
+const unsigned WASM_SYMBOL_VISIBILITY_HIDDEN = 0x4;
+const unsigned WASM_SYMBOL_UNDEFINED = 0x10;
+const unsigned WASM_SYMBOL_EXPORTED = 0x20;
+const unsigned WASM_SYMBOL_EXPLICIT_NAME = 0x40;
+const unsigned WASM_SYMBOL_NO_STRIP = 0x80;
+const unsigned WASM_SYMBOL_TLS = 0x100;
+const unsigned WASM_SYMBOL_ABSOLUTE = 0x200;
+
+#define WASM_RELOC(name, value) name = value,
+
+enum : unsigned {
+#include "WasmRelocs.def"
+};
+
+#undef WASM_RELOC
+
 struct WasmObjectHeader {
   StringRef Magic;
   uint32_t Version;
 };
 
+// Subset of types that a value can have
+enum class ValType {
+  I32 = WASM_TYPE_I32,
+  I64 = WASM_TYPE_I64,
+  F32 = WASM_TYPE_F32,
+  F64 = WASM_TYPE_F64,
+  V128 = WASM_TYPE_V128,
+  FUNCREF = WASM_TYPE_FUNCREF,
+  EXTERNREF = WASM_TYPE_EXTERNREF,
+};
+
 struct WasmDylinkImportInfo {
   StringRef Module;
   StringRef Field;
@@ -80,8 +276,9 @@ struct WasmLimits {
   uint64_t Maximum;
 };
 
+
 struct WasmTableType {
-  uint8_t ElemType;
+  ValType ElemType;
   WasmLimits Limits;
 };
 
@@ -110,7 +307,7 @@ struct WasmInitExpr {
 };
 
 struct WasmGlobalType {
-  uint8_t Type;
+  uint8_t Type; // TODO: make this a ValType?
   bool Mutable;
 };
 
@@ -175,7 +372,7 @@ struct WasmDataSegment {
 struct WasmElemSegment {
   uint32_t Flags;
   uint32_t TableNumber;
-  uint8_t ElemKind;
+  ValType ElemKind;
   WasmInitExpr Offset;
   std::vector<uint32_t> Functions;
 };
@@ -238,201 +435,6 @@ struct WasmLinkingData {
   std::vector<WasmSymbolInfo> SymbolTable;
 };
 
-enum : unsigned {
-  WASM_SEC_CUSTOM = 0,     // Custom / User-defined section
-  WASM_SEC_TYPE = 1,       // Function signature declarations
-  WASM_SEC_IMPORT = 2,     // Import declarations
-  WASM_SEC_FUNCTION = 3,   // Function declarations
-  WASM_SEC_TABLE = 4,      // Indirect function table and other tables
-  WASM_SEC_MEMORY = 5,     // Memory attributes
-  WASM_SEC_GLOBAL = 6,     // Global declarations
-  WASM_SEC_EXPORT = 7,     // Exports
-  WASM_SEC_START = 8,      // Start function declaration
-  WASM_SEC_ELEM = 9,       // Elements section
-  WASM_SEC_CODE = 10,      // Function bodies (code)
-  WASM_SEC_DATA = 11,      // Data segments
-  WASM_SEC_DATACOUNT = 12, // Data segment count
-  WASM_SEC_TAG = 13,       // Tag declarations
-  WASM_SEC_LAST_KNOWN = WASM_SEC_TAG,
-};
-
-// Type immediate encodings used in various contexts.
-enum : unsigned {
-  WASM_TYPE_I32 = 0x7F,
-  WASM_TYPE_I64 = 0x7E,
-  WASM_TYPE_F32 = 0x7D,
-  WASM_TYPE_F64 = 0x7C,
-  WASM_TYPE_V128 = 0x7B,
-  WASM_TYPE_FUNCREF = 0x70,
-  WASM_TYPE_EXTERNREF = 0x6F,
-  WASM_TYPE_FUNC = 0x60,
-  WASM_TYPE_NORESULT = 0x40, // for blocks with no result values
-};
-
-// Kinds of externals (for imports and exports).
-enum : unsigned {
-  WASM_EXTERNAL_FUNCTION = 0x0,
-  WASM_EXTERNAL_TABLE = 0x1,
-  WASM_EXTERNAL_MEMORY = 0x2,
-  WASM_EXTERNAL_GLOBAL = 0x3,
-  WASM_EXTERNAL_TAG = 0x4,
-};
-
-// Opcodes used in initializer expressions.
-enum : unsigned {
-  WASM_OPCODE_END = 0x0b,
-  WASM_OPCODE_CALL = 0x10,
-  WASM_OPCODE_LOCAL_GET = 0x20,
-  WASM_OPCODE_LOCAL_SET = 0x21,
-  WASM_OPCODE_LOCAL_TEE = 0x22,
-  WASM_OPCODE_GLOBAL_GET = 0x23,
-  WASM_OPCODE_GLOBAL_SET = 0x24,
-  WASM_OPCODE_I32_STORE = 0x36,
-  WASM_OPCODE_I64_STORE = 0x37,
-  WASM_OPCODE_I32_CONST = 0x41,
-  WASM_OPCODE_I64_CONST = 0x42,
-  WASM_OPCODE_F32_CONST = 0x43,
-  WASM_OPCODE_F64_CONST = 0x44,
-  WASM_OPCODE_I32_ADD = 0x6a,
-  WASM_OPCODE_I32_SUB = 0x6b,
-  WASM_OPCODE_I32_MUL = 0x6c,
-  WASM_OPCODE_I64_ADD = 0x7c,
-  WASM_OPCODE_I64_SUB = 0x7d,
-  WASM_OPCODE_I64_MUL = 0x7e,
-  WASM_OPCODE_REF_NULL = 0xd0,
-};
-
-// Opcodes used in synthetic functions.
-enum : unsigned {
-  WASM_OPCODE_BLOCK = 0x02,
-  WASM_OPCODE_BR = 0x0c,
-  WASM_OPCODE_BR_TABLE = 0x0e,
-  WASM_OPCODE_RETURN = 0x0f,
-  WASM_OPCODE_DROP = 0x1a,
-  WASM_OPCODE_MISC_PREFIX = 0xfc,
-  WASM_OPCODE_MEMORY_INIT = 0x08,
-  WASM_OPCODE_MEMORY_FILL = 0x0b,
-  WASM_OPCODE_DATA_DROP = 0x09,
-  WASM_OPCODE_ATOMICS_PREFIX = 0xfe,
-  WASM_OPCODE_ATOMIC_NOTIFY = 0x00,
-  WASM_OPCODE_I32_ATOMIC_WAIT = 0x01,
-  WASM_OPCODE_I32_ATOMIC_STORE = 0x17,
-  WASM_OPCODE_I32_RMW_CMPXCHG = 0x48,
-};
-
-enum : unsigned {
-  WASM_LIMITS_FLAG_NONE = 0x0,
-  WASM_LIMITS_FLAG_HAS_MAX = 0x1,
-  WASM_LIMITS_FLAG_IS_SHARED = 0x2,
-  WASM_LIMITS_FLAG_IS_64 = 0x4,
-};
-
-enum : unsigned {
-  WASM_DATA_SEGMENT_IS_PASSIVE = 0x01,
-  WASM_DATA_SEGMENT_HAS_MEMINDEX = 0x02,
-};
-
-enum : unsigned {
-  WASM_ELEM_SEGMENT_IS_PASSIVE = 0x01,
-  WASM_ELEM_SEGMENT_HAS_TABLE_NUMBER = 0x02,
-  WASM_ELEM_SEGMENT_HAS_INIT_EXPRS = 0x04,
-};
-const unsigned WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND = 0x3;
-
-// Feature policy prefixes used in the custom "target_features" section
-enum : uint8_t {
-  WASM_FEATURE_PREFIX_USED = '+',
-  WASM_FEATURE_PREFIX_REQUIRED = '=',
-  WASM_FEATURE_PREFIX_DISALLOWED = '-',
-};
-
-// Kind codes used in the custom "name" section
-enum : unsigned {
-  WASM_NAMES_MODULE = 0,
-  WASM_NAMES_FUNCTION = 1,
-  WASM_NAMES_LOCAL = 2,
-  WASM_NAMES_GLOBAL = 7,
-  WASM_NAMES_DATA_SEGMENT = 9,
-};
-
-// Kind codes used in the custom "linking" section
-enum : unsigned {
-  WASM_SEGMENT_INFO = 0x5,
-  WASM_INIT_FUNCS = 0x6,
-  WASM_COMDAT_INFO = 0x7,
-  WASM_SYMBOL_TABLE = 0x8,
-};
-
-// Kind codes used in the custom "dylink" section
-enum : unsigned {
-  WASM_DYLINK_MEM_INFO = 0x1,
-  WASM_DYLINK_NEEDED = 0x2,
-  WASM_DYLINK_EXPORT_INFO = 0x3,
-  WASM_DYLINK_IMPORT_INFO = 0x4,
-};
-
-// Kind codes used in the custom "linking" section in the WASM_COMDAT_INFO
-enum : unsigned {
-  WASM_COMDAT_DATA = 0x0,
-  WASM_COMDAT_FUNCTION = 0x1,
-  // GLOBAL, TAG, and TABLE are in here but LLVM doesn't use them yet.
-  WASM_COMDAT_SECTION = 0x5,
-};
-
-// Kind codes used in the custom "linking" section in the WASM_SYMBOL_TABLE
-enum WasmSymbolType : unsigned {
-  WASM_SYMBOL_TYPE_FUNCTION = 0x0,
-  WASM_SYMBOL_TYPE_DATA = 0x1,
-  WASM_SYMBOL_TYPE_GLOBAL = 0x2,
-  WASM_SYMBOL_TYPE_SECTION = 0x3,
-  WASM_SYMBOL_TYPE_TAG = 0x4,
-  WASM_SYMBOL_TYPE_TABLE = 0x5,
-};
-
-enum WasmSegmentFlag : unsigned {
-  WASM_SEG_FLAG_STRINGS = 0x1,
-  WASM_SEG_FLAG_TLS = 0x2,
-};
-
-// Kinds of tag attributes.
-enum WasmTagAttribute : uint8_t {
-  WASM_TAG_ATTRIBUTE_EXCEPTION = 0x0,
-};
-
-const unsigned WASM_SYMBOL_BINDING_MASK = 0x3;
-const unsigned WASM_SYMBOL_VISIBILITY_MASK = 0xc;
-
-const unsigned WASM_SYMBOL_BINDING_GLOBAL = 0x0;
-const unsigned WASM_SYMBOL_BINDING_WEAK = 0x1;
-const unsigned WASM_SYMBOL_BINDING_LOCAL = 0x2;
-const unsigned WASM_SYMBOL_VISIBILITY_DEFAULT = 0x0;
-const unsigned WASM_SYMBOL_VISIBILITY_HIDDEN = 0x4;
-const unsigned WASM_SYMBOL_UNDEFINED = 0x10;
-const unsigned WASM_SYMBOL_EXPORTED = 0x20;
-const unsigned WASM_SYMBOL_EXPLICIT_NAME = 0x40;
-const unsigned WASM_SYMBOL_NO_STRIP = 0x80;
-const unsigned WASM_SYMBOL_TLS = 0x100;
-const unsigned WASM_SYMBOL_ABSOLUTE = 0x200;
-
-#define WASM_RELOC(name, value) name = value,
-
-enum : unsigned {
-#include "WasmRelocs.def"
-};
-
-#undef WASM_RELOC
-
-// Subset of types that a value can have
-enum class ValType {
-  I32 = WASM_TYPE_I32,
-  I64 = WASM_TYPE_I64,
-  F32 = WASM_TYPE_F32,
-  F64 = WASM_TYPE_F64,
-  V128 = WASM_TYPE_V128,
-  FUNCREF = WASM_TYPE_FUNCREF,
-  EXTERNREF = WASM_TYPE_EXTERNREF,
-};
-
 struct WasmSignature {
   SmallVector<ValType, 1> Returns;
   SmallVector<ValType, 4> Params;
diff --git a/llvm/include/llvm/BinaryFormat/WasmTraits.h b/llvm/include/llvm/BinaryFormat/WasmTraits.h
index bef9dd3291ca76e..a279ce37a791ed9 100644
--- a/llvm/include/llvm/BinaryFormat/WasmTraits.h
+++ b/llvm/include/llvm/BinaryFormat/WasmTraits.h
@@ -87,11 +87,11 @@ template <> struct DenseMapInfo<wasm::WasmLimits, void> {
 template <> struct DenseMapInfo<wasm::WasmTableType, void> {
   static wasm::WasmTableType getEmptyKey() {
     return wasm::WasmTableType{
-        0, DenseMapInfo<wasm::WasmLimits, void>::getEmptyKey()};
+        wasm::ValType(0), DenseMapInfo<wasm::WasmLimits, void>::getEmptyKey()};
   }
   static wasm::WasmTableType getTombstoneKey() {
     return wasm::WasmTableType{
-        1, DenseMapInfo<wasm::WasmLimits, void>::getTombstoneKey()};
+        wasm::ValType(1), DenseMapInfo<wasm::WasmLimits, void>::getTombstoneKey()};
   }
   static unsigned getHashValue(const wasm::WasmTableType &TableType) {
     return hash_combine(
diff --git a/llvm/include/llvm/MC/MCSymbolWasm.h b/llvm/include/llvm/MC/MCSymbolWasm.h
index c67bd64e7cbdfda..0ce95c72a73f702 100644
--- a/llvm/include/llvm/MC/MCSymbolWasm.h
+++ b/llvm/include/llvm/MC/MCSymbolWasm.h
@@ -112,7 +112,7 @@ class MCSymbolWasm : public MCSymbol {
 
   bool isFunctionTable() const {
     return isTable() && hasTableType() &&
-           getTableType().ElemType == wasm::WASM_TYPE_FUNCREF;
+           getTableType().ElemType == wasm::ValType::FUNCREF;
   }
   void setFunctionTable() {
     setType(wasm::WASM_SYMBOL_TYPE_TABLE);
@@ -144,7 +144,7 @@ class MCSymbolWasm : public MCSymbol {
     // 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});
+    setTableType({VT, Limits});
   }
 };
 
diff --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp
index e43f111113b408f..ead4f9eff4c8512 100644
--- a/llvm/lib/MC/WasmObjectWriter.cpp
+++ b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -972,7 +972,7 @@ void WasmObjectWriter::writeTableSection(ArrayRef<wasm::WasmTable> Tables) {
 
   encodeULEB128(Tables.size(), W->OS);
   for (const wasm::WasmTable &Table : Tables) {
-    encodeULEB128(Table.Type.ElemType, W->OS);
+    encodeULEB128((uint32_t)Table.Type.ElemType, W->OS);
     encodeULEB128(Table.Type.Limits.Flags, W->OS);
     encodeULEB128(Table.Type.Limits.Minimum, W->OS);
     if (Table.Type.Limits.Flags & wasm::WASM_LIMITS_FLAG_HAS_MAX)
diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index 94cd96968ff2010..b9a8e970216ba68 100644
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -258,7 +258,7 @@ static wasm::WasmLimits readLimits(WasmObjectFile::ReadContext &Ctx) {
 
 static wasm::WasmTableType readTableType(WasmObjectFile::ReadContext &Ctx) {
   wasm::WasmTableType TableType;
-  TableType.ElemType = readUint8(Ctx);
+  TableType.ElemType = wasm::ValType(readVaruint32(Ctx));
   TableType.Limits = readLimits(Ctx);
   return TableType;
 }
@@ -1163,8 +1163,8 @@ Error WasmObjectFile::parseImportSection(ReadContext &Ctx) {
       Im.Table = readTableType(Ctx);
       NumImportedTables++;
       auto ElemType = Im.Table.ElemType;
-      if (ElemType != wasm::WASM_TYPE_FUNCREF &&
-          ElemType != wasm::WASM_TYPE_EXTERNREF)
+      if (ElemType != wasm::ValType::FUNCREF &&
+          ElemType != wasm::ValType::EXTERNREF)
         return make_error<GenericBinaryError>("invalid table element type",
                                               object_error::parse_failed);
       break;
@@ -1220,8 +1220,8 @@ Error WasmObjectFile::parseTableSection(ReadContext &Ctx) {
     T.Index = NumImportedTables + Tables.size();
     Tables.push_back(T);
     auto ElemType = Tables.back().Type.ElemType;
-    if (ElemType != wasm::WASM_TYPE_FUNCREF &&
-        ElemType != wasm::WASM_TYPE_EXTERNREF) {
+    if (ElemType != wasm::ValType::FUNCREF &&
+        ElemType != wasm::ValType::EXTERNREF) {
       return make_error<GenericBinaryError>("invalid table element type",
                                             object_error::parse_failed);
     }
@@ -1534,21 +1534,22 @@ Error WasmObjectFile::parseElemSection(ReadContext &Ctx) {
     }
 
     if (Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND) {
-      Segment.ElemKind = readUint8(Ctx);
+      auto ElemKind = readVaruint32(Ctx);
       if (Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS) {
-        if (Segment.ElemKind != uint8_t(wasm::ValType::FUNCREF) &&
-            Segment.ElemKind != uint8_t(wasm::ValType::EXTERNREF)) {
+        Segment.ElemKind = wasm::ValType(ElemKind);
+        if (Segment.ElemKind != wasm::ValType::FUNCREF &&
+            Segment.ElemKind != wasm::ValType::EXTERNREF) {
           return make_error<GenericBinaryError>("invalid reference type",
                                                 object_error::parse_failed);
         }
       } else {
-        if (Segment.ElemKind != 0)
+        if (ElemKind != 0)
           return make_error<GenericBinaryError>("invalid elemtype",
                                                 object_error::parse_failed);
-        Segment.ElemKind = uint8_t(wasm::ValType::FUNCREF);
+        Segment.ElemKind = wasm::ValType::FUNCREF;
       }
     } else {
-      Segment.ElemKind = uint8_t(wasm::ValType::FUNCREF);
+      Segment.ElemKind = wasm::ValType::FUNCREF;
     }
 
     if (Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS)
diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
index 1b92997f03f11c9..3cc4d50271eb114 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -851,7 +851,7 @@ class WebAs...
[truncated]

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 13, 2024

@llvm/pr-subscribers-llvm-binary-utilities

Author: Derek Schuff (dschuff)

Changes

LLVM models some features found in the binary format with raw integers and others with nested or enumerated types. This PR switches modeling of tables and segments to use wasm::ValType rather than uint32_t. This NFC change is in preparation for modeling more reference types, but IMO is also cleaner and closer to the spec.


Patch is 22.00 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/78012.diff

10 Files Affected:

  • (modified) lld/wasm/InputFiles.cpp (+1-1)
  • (modified) lld/wasm/SymbolTable.cpp (+2-2)
  • (modified) llvm/include/llvm/BinaryFormat/Wasm.h (+200-198)
  • (modified) llvm/include/llvm/BinaryFormat/WasmTraits.h (+2-2)
  • (modified) llvm/include/llvm/MC/MCSymbolWasm.h (+2-2)
  • (modified) llvm/lib/MC/WasmObjectWriter.cpp (+1-1)
  • (modified) llvm/lib/Object/WasmObjectFile.cpp (+12-11)
  • (modified) llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp (+1-1)
  • (modified) llvm/lib/Target/WebAssembly/WebAssemblyUtilities.cpp (+1-1)
  • (modified) llvm/tools/obj2yaml/wasm2yaml.cpp (+2-2)
diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp
index 5709a5ced584c7..97c88587231ba9 100644
--- a/lld/wasm/InputFiles.cpp
+++ b/lld/wasm/InputFiles.cpp
@@ -320,7 +320,7 @@ void ObjFile::addLegacyIndirectFunctionTableIfNeeded(
   // it has an unexpected name or type, assume that it's not actually the
   // indirect function table.
   if (tableImport->Field != functionTableName ||
-      tableImport->Table.ElemType != uint8_t(ValType::FUNCREF)) {
+      tableImport->Table.ElemType != ValType::FUNCREF) {
     error(toString(this) + ": table import " + Twine(tableImport->Field) +
           " is missing a symbol table entry.");
     return;
diff --git a/lld/wasm/SymbolTable.cpp b/lld/wasm/SymbolTable.cpp
index 76370525c37199..b5c138cd76392d 100644
--- a/lld/wasm/SymbolTable.cpp
+++ b/lld/wasm/SymbolTable.cpp
@@ -676,7 +676,7 @@ Symbol *SymbolTable::addUndefinedTag(StringRef name,
 TableSymbol *SymbolTable::createUndefinedIndirectFunctionTable(StringRef name) {
   WasmLimits limits{0, 0, 0}; // Set by the writer.
   WasmTableType *type = make<WasmTableType>();
-  type->ElemType = uint8_t(ValType::FUNCREF);
+  type->ElemType = ValType::FUNCREF;
   type->Limits = limits;
   StringRef module(defaultModule);
   uint32_t flags = config->exportTable ? 0 : WASM_SYMBOL_VISIBILITY_HIDDEN;
@@ -690,7 +690,7 @@ TableSymbol *SymbolTable::createUndefinedIndirectFunctionTable(StringRef name) {
 TableSymbol *SymbolTable::createDefinedIndirectFunctionTable(StringRef name) {
   const uint32_t invalidIndex = -1;
   WasmLimits limits{0, 0, 0}; // Set by the writer.
-  WasmTableType type{uint8_t(ValType::FUNCREF), limits};
+  WasmTableType type{ValType::FUNCREF, limits};
   WasmTable desc{invalidIndex, type, name};
   InputTable *table = make<InputTable>(desc, nullptr);
   uint32_t flags = config->exportTable ? 0 : WASM_SYMBOL_VISIBILITY_HIDDEN;
diff --git a/llvm/include/llvm/BinaryFormat/Wasm.h b/llvm/include/llvm/BinaryFormat/Wasm.h
index c7658cc7b7432b..9921af5e52a733 100644
--- a/llvm/include/llvm/BinaryFormat/Wasm.h
+++ b/llvm/include/llvm/BinaryFormat/Wasm.h
@@ -31,11 +31,207 @@ const uint32_t WasmMetadataVersion = 0x2;
 // Wasm uses a 64k page size
 const uint32_t WasmPageSize = 65536;
 
+enum : unsigned {
+  WASM_SEC_CUSTOM = 0,     // Custom / User-defined section
+  WASM_SEC_TYPE = 1,       // Function signature declarations
+  WASM_SEC_IMPORT = 2,     // Import declarations
+  WASM_SEC_FUNCTION = 3,   // Function declarations
+  WASM_SEC_TABLE = 4,      // Indirect function table and other tables
+  WASM_SEC_MEMORY = 5,     // Memory attributes
+  WASM_SEC_GLOBAL = 6,     // Global declarations
+  WASM_SEC_EXPORT = 7,     // Exports
+  WASM_SEC_START = 8,      // Start function declaration
+  WASM_SEC_ELEM = 9,       // Elements section
+  WASM_SEC_CODE = 10,      // Function bodies (code)
+  WASM_SEC_DATA = 11,      // Data segments
+  WASM_SEC_DATACOUNT = 12, // Data segment count
+  WASM_SEC_TAG = 13,       // Tag declarations
+  WASM_SEC_LAST_KNOWN = WASM_SEC_TAG,
+};
+
+// Type immediate encodings used in various contexts.
+enum : unsigned {
+  WASM_TYPE_I32 = 0x7F,
+  WASM_TYPE_I64 = 0x7E,
+  WASM_TYPE_F32 = 0x7D,
+  WASM_TYPE_F64 = 0x7C,
+  WASM_TYPE_V128 = 0x7B,
+  WASM_TYPE_FUNCREF = 0x70,
+  WASM_TYPE_EXTERNREF = 0x6F,
+  WASM_TYPE_FUNC = 0x60,
+  WASM_TYPE_NORESULT = 0x40, // for blocks with no result values
+};
+
+// Kinds of externals (for imports and exports).
+enum : unsigned {
+  WASM_EXTERNAL_FUNCTION = 0x0,
+  WASM_EXTERNAL_TABLE = 0x1,
+  WASM_EXTERNAL_MEMORY = 0x2,
+  WASM_EXTERNAL_GLOBAL = 0x3,
+  WASM_EXTERNAL_TAG = 0x4,
+};
+
+// Opcodes used in initializer expressions.
+enum : unsigned {
+  WASM_OPCODE_END = 0x0b,
+  WASM_OPCODE_CALL = 0x10,
+  WASM_OPCODE_LOCAL_GET = 0x20,
+  WASM_OPCODE_LOCAL_SET = 0x21,
+  WASM_OPCODE_LOCAL_TEE = 0x22,
+  WASM_OPCODE_GLOBAL_GET = 0x23,
+  WASM_OPCODE_GLOBAL_SET = 0x24,
+  WASM_OPCODE_I32_STORE = 0x36,
+  WASM_OPCODE_I64_STORE = 0x37,
+  WASM_OPCODE_I32_CONST = 0x41,
+  WASM_OPCODE_I64_CONST = 0x42,
+  WASM_OPCODE_F32_CONST = 0x43,
+  WASM_OPCODE_F64_CONST = 0x44,
+  WASM_OPCODE_I32_ADD = 0x6a,
+  WASM_OPCODE_I32_SUB = 0x6b,
+  WASM_OPCODE_I32_MUL = 0x6c,
+  WASM_OPCODE_I64_ADD = 0x7c,
+  WASM_OPCODE_I64_SUB = 0x7d,
+  WASM_OPCODE_I64_MUL = 0x7e,
+  WASM_OPCODE_REF_NULL = 0xd0,
+};
+
+
+// Opcodes used in synthetic functions.
+enum : unsigned {
+  WASM_OPCODE_BLOCK = 0x02,
+  WASM_OPCODE_BR = 0x0c,
+  WASM_OPCODE_BR_TABLE = 0x0e,
+  WASM_OPCODE_RETURN = 0x0f,
+  WASM_OPCODE_DROP = 0x1a,
+  WASM_OPCODE_MISC_PREFIX = 0xfc,
+  WASM_OPCODE_MEMORY_INIT = 0x08,
+  WASM_OPCODE_MEMORY_FILL = 0x0b,
+  WASM_OPCODE_DATA_DROP = 0x09,
+  WASM_OPCODE_ATOMICS_PREFIX = 0xfe,
+  WASM_OPCODE_ATOMIC_NOTIFY = 0x00,
+  WASM_OPCODE_I32_ATOMIC_WAIT = 0x01,
+  WASM_OPCODE_I32_ATOMIC_STORE = 0x17,
+  WASM_OPCODE_I32_RMW_CMPXCHG = 0x48,
+};
+
+enum : unsigned {
+  WASM_LIMITS_FLAG_NONE = 0x0,
+  WASM_LIMITS_FLAG_HAS_MAX = 0x1,
+  WASM_LIMITS_FLAG_IS_SHARED = 0x2,
+  WASM_LIMITS_FLAG_IS_64 = 0x4,
+};
+
+enum : unsigned {
+  WASM_DATA_SEGMENT_IS_PASSIVE = 0x01,
+  WASM_DATA_SEGMENT_HAS_MEMINDEX = 0x02,
+};
+
+enum : unsigned {
+  WASM_ELEM_SEGMENT_IS_PASSIVE = 0x01,
+  WASM_ELEM_SEGMENT_HAS_TABLE_NUMBER = 0x02,
+  WASM_ELEM_SEGMENT_HAS_INIT_EXPRS = 0x04,
+};
+const unsigned WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND = 0x3;
+
+// Feature policy prefixes used in the custom "target_features" section
+enum : uint8_t {
+  WASM_FEATURE_PREFIX_USED = '+',
+  WASM_FEATURE_PREFIX_REQUIRED = '=',
+  WASM_FEATURE_PREFIX_DISALLOWED = '-',
+};
+
+// Kind codes used in the custom "name" section
+enum : unsigned {
+  WASM_NAMES_MODULE = 0,
+  WASM_NAMES_FUNCTION = 1,
+  WASM_NAMES_LOCAL = 2,
+  WASM_NAMES_GLOBAL = 7,
+  WASM_NAMES_DATA_SEGMENT = 9,
+};
+
+// Kind codes used in the custom "linking" section
+enum : unsigned {
+  WASM_SEGMENT_INFO = 0x5,
+  WASM_INIT_FUNCS = 0x6,
+  WASM_COMDAT_INFO = 0x7,
+  WASM_SYMBOL_TABLE = 0x8,
+};
+
+// Kind codes used in the custom "dylink" section
+enum : unsigned {
+  WASM_DYLINK_MEM_INFO = 0x1,
+  WASM_DYLINK_NEEDED = 0x2,
+  WASM_DYLINK_EXPORT_INFO = 0x3,
+  WASM_DYLINK_IMPORT_INFO = 0x4,
+};
+
+// Kind codes used in the custom "linking" section in the WASM_COMDAT_INFO
+enum : unsigned {
+  WASM_COMDAT_DATA = 0x0,
+  WASM_COMDAT_FUNCTION = 0x1,
+  // GLOBAL, TAG, and TABLE are in here but LLVM doesn't use them yet.
+  WASM_COMDAT_SECTION = 0x5,
+};
+
+// Kind codes used in the custom "linking" section in the WASM_SYMBOL_TABLE
+enum WasmSymbolType : unsigned {
+  WASM_SYMBOL_TYPE_FUNCTION = 0x0,
+  WASM_SYMBOL_TYPE_DATA = 0x1,
+  WASM_SYMBOL_TYPE_GLOBAL = 0x2,
+  WASM_SYMBOL_TYPE_SECTION = 0x3,
+  WASM_SYMBOL_TYPE_TAG = 0x4,
+  WASM_SYMBOL_TYPE_TABLE = 0x5,
+};
+
+enum WasmSegmentFlag : unsigned {
+  WASM_SEG_FLAG_STRINGS = 0x1,
+  WASM_SEG_FLAG_TLS = 0x2,
+};
+
+// Kinds of tag attributes.
+enum WasmTagAttribute : uint8_t {
+  WASM_TAG_ATTRIBUTE_EXCEPTION = 0x0,
+};
+
+const unsigned WASM_SYMBOL_BINDING_MASK = 0x3;
+const unsigned WASM_SYMBOL_VISIBILITY_MASK = 0xc;
+
+const unsigned WASM_SYMBOL_BINDING_GLOBAL = 0x0;
+const unsigned WASM_SYMBOL_BINDING_WEAK = 0x1;
+const unsigned WASM_SYMBOL_BINDING_LOCAL = 0x2;
+const unsigned WASM_SYMBOL_VISIBILITY_DEFAULT = 0x0;
+const unsigned WASM_SYMBOL_VISIBILITY_HIDDEN = 0x4;
+const unsigned WASM_SYMBOL_UNDEFINED = 0x10;
+const unsigned WASM_SYMBOL_EXPORTED = 0x20;
+const unsigned WASM_SYMBOL_EXPLICIT_NAME = 0x40;
+const unsigned WASM_SYMBOL_NO_STRIP = 0x80;
+const unsigned WASM_SYMBOL_TLS = 0x100;
+const unsigned WASM_SYMBOL_ABSOLUTE = 0x200;
+
+#define WASM_RELOC(name, value) name = value,
+
+enum : unsigned {
+#include "WasmRelocs.def"
+};
+
+#undef WASM_RELOC
+
 struct WasmObjectHeader {
   StringRef Magic;
   uint32_t Version;
 };
 
+// Subset of types that a value can have
+enum class ValType {
+  I32 = WASM_TYPE_I32,
+  I64 = WASM_TYPE_I64,
+  F32 = WASM_TYPE_F32,
+  F64 = WASM_TYPE_F64,
+  V128 = WASM_TYPE_V128,
+  FUNCREF = WASM_TYPE_FUNCREF,
+  EXTERNREF = WASM_TYPE_EXTERNREF,
+};
+
 struct WasmDylinkImportInfo {
   StringRef Module;
   StringRef Field;
@@ -80,8 +276,9 @@ struct WasmLimits {
   uint64_t Maximum;
 };
 
+
 struct WasmTableType {
-  uint8_t ElemType;
+  ValType ElemType;
   WasmLimits Limits;
 };
 
@@ -110,7 +307,7 @@ struct WasmInitExpr {
 };
 
 struct WasmGlobalType {
-  uint8_t Type;
+  uint8_t Type; // TODO: make this a ValType?
   bool Mutable;
 };
 
@@ -175,7 +372,7 @@ struct WasmDataSegment {
 struct WasmElemSegment {
   uint32_t Flags;
   uint32_t TableNumber;
-  uint8_t ElemKind;
+  ValType ElemKind;
   WasmInitExpr Offset;
   std::vector<uint32_t> Functions;
 };
@@ -238,201 +435,6 @@ struct WasmLinkingData {
   std::vector<WasmSymbolInfo> SymbolTable;
 };
 
-enum : unsigned {
-  WASM_SEC_CUSTOM = 0,     // Custom / User-defined section
-  WASM_SEC_TYPE = 1,       // Function signature declarations
-  WASM_SEC_IMPORT = 2,     // Import declarations
-  WASM_SEC_FUNCTION = 3,   // Function declarations
-  WASM_SEC_TABLE = 4,      // Indirect function table and other tables
-  WASM_SEC_MEMORY = 5,     // Memory attributes
-  WASM_SEC_GLOBAL = 6,     // Global declarations
-  WASM_SEC_EXPORT = 7,     // Exports
-  WASM_SEC_START = 8,      // Start function declaration
-  WASM_SEC_ELEM = 9,       // Elements section
-  WASM_SEC_CODE = 10,      // Function bodies (code)
-  WASM_SEC_DATA = 11,      // Data segments
-  WASM_SEC_DATACOUNT = 12, // Data segment count
-  WASM_SEC_TAG = 13,       // Tag declarations
-  WASM_SEC_LAST_KNOWN = WASM_SEC_TAG,
-};
-
-// Type immediate encodings used in various contexts.
-enum : unsigned {
-  WASM_TYPE_I32 = 0x7F,
-  WASM_TYPE_I64 = 0x7E,
-  WASM_TYPE_F32 = 0x7D,
-  WASM_TYPE_F64 = 0x7C,
-  WASM_TYPE_V128 = 0x7B,
-  WASM_TYPE_FUNCREF = 0x70,
-  WASM_TYPE_EXTERNREF = 0x6F,
-  WASM_TYPE_FUNC = 0x60,
-  WASM_TYPE_NORESULT = 0x40, // for blocks with no result values
-};
-
-// Kinds of externals (for imports and exports).
-enum : unsigned {
-  WASM_EXTERNAL_FUNCTION = 0x0,
-  WASM_EXTERNAL_TABLE = 0x1,
-  WASM_EXTERNAL_MEMORY = 0x2,
-  WASM_EXTERNAL_GLOBAL = 0x3,
-  WASM_EXTERNAL_TAG = 0x4,
-};
-
-// Opcodes used in initializer expressions.
-enum : unsigned {
-  WASM_OPCODE_END = 0x0b,
-  WASM_OPCODE_CALL = 0x10,
-  WASM_OPCODE_LOCAL_GET = 0x20,
-  WASM_OPCODE_LOCAL_SET = 0x21,
-  WASM_OPCODE_LOCAL_TEE = 0x22,
-  WASM_OPCODE_GLOBAL_GET = 0x23,
-  WASM_OPCODE_GLOBAL_SET = 0x24,
-  WASM_OPCODE_I32_STORE = 0x36,
-  WASM_OPCODE_I64_STORE = 0x37,
-  WASM_OPCODE_I32_CONST = 0x41,
-  WASM_OPCODE_I64_CONST = 0x42,
-  WASM_OPCODE_F32_CONST = 0x43,
-  WASM_OPCODE_F64_CONST = 0x44,
-  WASM_OPCODE_I32_ADD = 0x6a,
-  WASM_OPCODE_I32_SUB = 0x6b,
-  WASM_OPCODE_I32_MUL = 0x6c,
-  WASM_OPCODE_I64_ADD = 0x7c,
-  WASM_OPCODE_I64_SUB = 0x7d,
-  WASM_OPCODE_I64_MUL = 0x7e,
-  WASM_OPCODE_REF_NULL = 0xd0,
-};
-
-// Opcodes used in synthetic functions.
-enum : unsigned {
-  WASM_OPCODE_BLOCK = 0x02,
-  WASM_OPCODE_BR = 0x0c,
-  WASM_OPCODE_BR_TABLE = 0x0e,
-  WASM_OPCODE_RETURN = 0x0f,
-  WASM_OPCODE_DROP = 0x1a,
-  WASM_OPCODE_MISC_PREFIX = 0xfc,
-  WASM_OPCODE_MEMORY_INIT = 0x08,
-  WASM_OPCODE_MEMORY_FILL = 0x0b,
-  WASM_OPCODE_DATA_DROP = 0x09,
-  WASM_OPCODE_ATOMICS_PREFIX = 0xfe,
-  WASM_OPCODE_ATOMIC_NOTIFY = 0x00,
-  WASM_OPCODE_I32_ATOMIC_WAIT = 0x01,
-  WASM_OPCODE_I32_ATOMIC_STORE = 0x17,
-  WASM_OPCODE_I32_RMW_CMPXCHG = 0x48,
-};
-
-enum : unsigned {
-  WASM_LIMITS_FLAG_NONE = 0x0,
-  WASM_LIMITS_FLAG_HAS_MAX = 0x1,
-  WASM_LIMITS_FLAG_IS_SHARED = 0x2,
-  WASM_LIMITS_FLAG_IS_64 = 0x4,
-};
-
-enum : unsigned {
-  WASM_DATA_SEGMENT_IS_PASSIVE = 0x01,
-  WASM_DATA_SEGMENT_HAS_MEMINDEX = 0x02,
-};
-
-enum : unsigned {
-  WASM_ELEM_SEGMENT_IS_PASSIVE = 0x01,
-  WASM_ELEM_SEGMENT_HAS_TABLE_NUMBER = 0x02,
-  WASM_ELEM_SEGMENT_HAS_INIT_EXPRS = 0x04,
-};
-const unsigned WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND = 0x3;
-
-// Feature policy prefixes used in the custom "target_features" section
-enum : uint8_t {
-  WASM_FEATURE_PREFIX_USED = '+',
-  WASM_FEATURE_PREFIX_REQUIRED = '=',
-  WASM_FEATURE_PREFIX_DISALLOWED = '-',
-};
-
-// Kind codes used in the custom "name" section
-enum : unsigned {
-  WASM_NAMES_MODULE = 0,
-  WASM_NAMES_FUNCTION = 1,
-  WASM_NAMES_LOCAL = 2,
-  WASM_NAMES_GLOBAL = 7,
-  WASM_NAMES_DATA_SEGMENT = 9,
-};
-
-// Kind codes used in the custom "linking" section
-enum : unsigned {
-  WASM_SEGMENT_INFO = 0x5,
-  WASM_INIT_FUNCS = 0x6,
-  WASM_COMDAT_INFO = 0x7,
-  WASM_SYMBOL_TABLE = 0x8,
-};
-
-// Kind codes used in the custom "dylink" section
-enum : unsigned {
-  WASM_DYLINK_MEM_INFO = 0x1,
-  WASM_DYLINK_NEEDED = 0x2,
-  WASM_DYLINK_EXPORT_INFO = 0x3,
-  WASM_DYLINK_IMPORT_INFO = 0x4,
-};
-
-// Kind codes used in the custom "linking" section in the WASM_COMDAT_INFO
-enum : unsigned {
-  WASM_COMDAT_DATA = 0x0,
-  WASM_COMDAT_FUNCTION = 0x1,
-  // GLOBAL, TAG, and TABLE are in here but LLVM doesn't use them yet.
-  WASM_COMDAT_SECTION = 0x5,
-};
-
-// Kind codes used in the custom "linking" section in the WASM_SYMBOL_TABLE
-enum WasmSymbolType : unsigned {
-  WASM_SYMBOL_TYPE_FUNCTION = 0x0,
-  WASM_SYMBOL_TYPE_DATA = 0x1,
-  WASM_SYMBOL_TYPE_GLOBAL = 0x2,
-  WASM_SYMBOL_TYPE_SECTION = 0x3,
-  WASM_SYMBOL_TYPE_TAG = 0x4,
-  WASM_SYMBOL_TYPE_TABLE = 0x5,
-};
-
-enum WasmSegmentFlag : unsigned {
-  WASM_SEG_FLAG_STRINGS = 0x1,
-  WASM_SEG_FLAG_TLS = 0x2,
-};
-
-// Kinds of tag attributes.
-enum WasmTagAttribute : uint8_t {
-  WASM_TAG_ATTRIBUTE_EXCEPTION = 0x0,
-};
-
-const unsigned WASM_SYMBOL_BINDING_MASK = 0x3;
-const unsigned WASM_SYMBOL_VISIBILITY_MASK = 0xc;
-
-const unsigned WASM_SYMBOL_BINDING_GLOBAL = 0x0;
-const unsigned WASM_SYMBOL_BINDING_WEAK = 0x1;
-const unsigned WASM_SYMBOL_BINDING_LOCAL = 0x2;
-const unsigned WASM_SYMBOL_VISIBILITY_DEFAULT = 0x0;
-const unsigned WASM_SYMBOL_VISIBILITY_HIDDEN = 0x4;
-const unsigned WASM_SYMBOL_UNDEFINED = 0x10;
-const unsigned WASM_SYMBOL_EXPORTED = 0x20;
-const unsigned WASM_SYMBOL_EXPLICIT_NAME = 0x40;
-const unsigned WASM_SYMBOL_NO_STRIP = 0x80;
-const unsigned WASM_SYMBOL_TLS = 0x100;
-const unsigned WASM_SYMBOL_ABSOLUTE = 0x200;
-
-#define WASM_RELOC(name, value) name = value,
-
-enum : unsigned {
-#include "WasmRelocs.def"
-};
-
-#undef WASM_RELOC
-
-// Subset of types that a value can have
-enum class ValType {
-  I32 = WASM_TYPE_I32,
-  I64 = WASM_TYPE_I64,
-  F32 = WASM_TYPE_F32,
-  F64 = WASM_TYPE_F64,
-  V128 = WASM_TYPE_V128,
-  FUNCREF = WASM_TYPE_FUNCREF,
-  EXTERNREF = WASM_TYPE_EXTERNREF,
-};
-
 struct WasmSignature {
   SmallVector<ValType, 1> Returns;
   SmallVector<ValType, 4> Params;
diff --git a/llvm/include/llvm/BinaryFormat/WasmTraits.h b/llvm/include/llvm/BinaryFormat/WasmTraits.h
index bef9dd3291ca76..a279ce37a791ed 100644
--- a/llvm/include/llvm/BinaryFormat/WasmTraits.h
+++ b/llvm/include/llvm/BinaryFormat/WasmTraits.h
@@ -87,11 +87,11 @@ template <> struct DenseMapInfo<wasm::WasmLimits, void> {
 template <> struct DenseMapInfo<wasm::WasmTableType, void> {
   static wasm::WasmTableType getEmptyKey() {
     return wasm::WasmTableType{
-        0, DenseMapInfo<wasm::WasmLimits, void>::getEmptyKey()};
+        wasm::ValType(0), DenseMapInfo<wasm::WasmLimits, void>::getEmptyKey()};
   }
   static wasm::WasmTableType getTombstoneKey() {
     return wasm::WasmTableType{
-        1, DenseMapInfo<wasm::WasmLimits, void>::getTombstoneKey()};
+        wasm::ValType(1), DenseMapInfo<wasm::WasmLimits, void>::getTombstoneKey()};
   }
   static unsigned getHashValue(const wasm::WasmTableType &TableType) {
     return hash_combine(
diff --git a/llvm/include/llvm/MC/MCSymbolWasm.h b/llvm/include/llvm/MC/MCSymbolWasm.h
index c67bd64e7cbdfd..0ce95c72a73f70 100644
--- a/llvm/include/llvm/MC/MCSymbolWasm.h
+++ b/llvm/include/llvm/MC/MCSymbolWasm.h
@@ -112,7 +112,7 @@ class MCSymbolWasm : public MCSymbol {
 
   bool isFunctionTable() const {
     return isTable() && hasTableType() &&
-           getTableType().ElemType == wasm::WASM_TYPE_FUNCREF;
+           getTableType().ElemType == wasm::ValType::FUNCREF;
   }
   void setFunctionTable() {
     setType(wasm::WASM_SYMBOL_TYPE_TABLE);
@@ -144,7 +144,7 @@ class MCSymbolWasm : public MCSymbol {
     // 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});
+    setTableType({VT, Limits});
   }
 };
 
diff --git a/llvm/lib/MC/WasmObjectWriter.cpp b/llvm/lib/MC/WasmObjectWriter.cpp
index e43f111113b408..ead4f9eff4c851 100644
--- a/llvm/lib/MC/WasmObjectWriter.cpp
+++ b/llvm/lib/MC/WasmObjectWriter.cpp
@@ -972,7 +972,7 @@ void WasmObjectWriter::writeTableSection(ArrayRef<wasm::WasmTable> Tables) {
 
   encodeULEB128(Tables.size(), W->OS);
   for (const wasm::WasmTable &Table : Tables) {
-    encodeULEB128(Table.Type.ElemType, W->OS);
+    encodeULEB128((uint32_t)Table.Type.ElemType, W->OS);
     encodeULEB128(Table.Type.Limits.Flags, W->OS);
     encodeULEB128(Table.Type.Limits.Minimum, W->OS);
     if (Table.Type.Limits.Flags & wasm::WASM_LIMITS_FLAG_HAS_MAX)
diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index 94cd96968ff201..b9a8e970216ba6 100644
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -258,7 +258,7 @@ static wasm::WasmLimits readLimits(WasmObjectFile::ReadContext &Ctx) {
 
 static wasm::WasmTableType readTableType(WasmObjectFile::ReadContext &Ctx) {
   wasm::WasmTableType TableType;
-  TableType.ElemType = readUint8(Ctx);
+  TableType.ElemType = wasm::ValType(readVaruint32(Ctx));
   TableType.Limits = readLimits(Ctx);
   return TableType;
 }
@@ -1163,8 +1163,8 @@ Error WasmObjectFile::parseImportSection(ReadContext &Ctx) {
       Im.Table = readTableType(Ctx);
       NumImportedTables++;
       auto ElemType = Im.Table.ElemType;
-      if (ElemType != wasm::WASM_TYPE_FUNCREF &&
-          ElemType != wasm::WASM_TYPE_EXTERNREF)
+      if (ElemType != wasm::ValType::FUNCREF &&
+          ElemType != wasm::ValType::EXTERNREF)
         return make_error<GenericBinaryError>("invalid table element type",
                                               object_error::parse_failed);
       break;
@@ -1220,8 +1220,8 @@ Error WasmObjectFile::parseTableSection(ReadContext &Ctx) {
     T.Index = NumImportedTables + Tables.size();
     Tables.push_back(T);
     auto ElemType = Tables.back().Type.ElemType;
-    if (ElemType != wasm::WASM_TYPE_FUNCREF &&
-        ElemType != wasm::WASM_TYPE_EXTERNREF) {
+    if (ElemType != wasm::ValType::FUNCREF &&
+        ElemType != wasm::ValType::EXTERNREF) {
       return make_error<GenericBinaryError>("invalid table element type",
                                             object_error::parse_failed);
     }
@@ -1534,21 +1534,22 @@ Error WasmObjectFile::parseElemSection(ReadContext &Ctx) {
     }
 
     if (Segment.Flags & wasm::WASM_ELEM_SEGMENT_MASK_HAS_ELEM_KIND) {
-      Segment.ElemKind = readUint8(Ctx);
+      auto ElemKind = readVaruint32(Ctx);
       if (Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS) {
-        if (Segment.ElemKind != uint8_t(wasm::ValType::FUNCREF) &&
-            Segment.ElemKind != uint8_t(wasm::ValType::EXTERNREF)) {
+        Segment.ElemKind = wasm::ValType(ElemKind);
+        if (Segment.ElemKind != wasm::ValType::FUNCREF &&
+            Segment.ElemKind != wasm::ValType::EXTERNREF) {
           return make_error<GenericBinaryError>("invalid reference type",
                                                 object_error::parse_failed);
         }
       } else {
-        if (Segment.ElemKind != 0)
+        if (ElemKind != 0)
           return make_error<GenericBinaryError>("invalid elemtype",
                                                 object_error::parse_failed);
-        Segment.ElemKind = uint8_t(wasm::ValType::FUNCREF);
+        Segment.ElemKind = wasm::ValType::FUNCREF;
       }
     } else {
-      Segment.ElemKind = uint8_t(wasm::ValType::FUNCREF);
+      Segment.ElemKind = wasm::ValType::FUNCREF;
     }
 
     if (Segment.Flags & wasm::WASM_ELEM_SEGMENT_HAS_INIT_EXPRS)
diff --git a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
index 1b92997f03f11c..3cc4d50271eb11 100644
--- a/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
+++ b/llvm/lib/Target/WebAssembly/AsmParser/WebAssemblyAsmParser.cpp
@@ -851,7 +851,7 @@ class WebAssemblyAsmParser ...
[truncated]

Copy link

github-actions bot commented Jan 13, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Collaborator

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I like how this removes a bunch of casts.

FUNCREF = WASM_TYPE_FUNCREF,
EXTERNREF = WASM_TYPE_EXTERNREF,
};

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this large code block just moved up? Is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, the numeric constants just moved up to the top because ValType uses them, and it now needs to go before WasmTableType and the others that use it. Strictly speaking it's not actually necessary for ValType to use the same constants, I think it was just done for convenience in the decoder.

@@ -258,7 +258,7 @@ static wasm::WasmLimits readLimits(WasmObjectFile::ReadContext &Ctx) {

static wasm::WasmTableType readTableType(WasmObjectFile::ReadContext &Ctx) {
wasm::WasmTableType TableType;
TableType.ElemType = readUint8(Ctx);
TableType.ElemType = wasm::ValType(readVaruint32(Ctx));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this changes from uint8 to uint32. Does that match the spec?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, although it's slightly more complicated. The spec does say that the encoding for valtype is LEB128 (https://webassembly.github.io/gc/core/binary/types.html#binary-valtype), although it also mentions that the numeric constants are selected to correspond to the encoding of negative numbers to differentiate between direct encoding and use of a type index. LLVM doesn't make use of that property though, and we use the positive hex forms of the constants and unsigned representation in the code. (whereas Binaryen goes the signed route). Also the spec says that all the direct encodings (number, vector, reference/heap types) all use single bytes, so uint8 could work too.

@dschuff
Copy link
Member Author

dschuff commented Jan 17, 2024

Weird that the 2 code formatter plugins seem to disagree. I think this is ready again now.

@dschuff dschuff merged commit 103fa32 into llvm:main Jan 17, 2024
4 checks passed
@dschuff dschuff deleted the tablevaltype branch January 17, 2024 19:29
ampandey-1995 pushed a commit to ampandey-1995/llvm-project that referenced this pull request Jan 19, 2024
…es (llvm#78012)

LLVM models some features found in the binary format with raw integers
and others with nested or enumerated types. This PR switches modeling of
tables and segments to use wasm::ValType rather than uint32_t. This NFC
change is in preparation for modeling more reference types, but IMO is
also cleaner and closer to the spec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants