[WebAssembly] Import the memory and table for .o files

[sunfish]

Summary

Instead of having .o files contain linear-memory and function table definitions, use imports. This is more consistent with the stack pointer being imported, and it's consistent with the linker being the one to decide whether linear memory and function table are imported or defined in the linked output. This implements tool-conventions #23.

Edited to remove WASM_DEFAULT_LINEAR_MEMORY and WASM_DEFAULT_FUNCTION_TABLE changes.

[ncw]

In the GitHub PR, it sounds like you've decided to keep WASM_DATA_SIZE after all, and get rid of WASM_STACK_POINTER/WASM_DEFAULT_LINEAR_MEMORY/WASM_DEFAULT_TABLE in favour of using the symbol names?

[sunfish]

• Remove the WASM_DEFAULT_LINEAR_MEMORY and WASM_DEFAULT_FUNCTION_TABLE metadata.
• Update test/MC tests.

[sbc100]

Can you update the description.

Code lgtm. But I'm on the fence as to weather this change is really making any useful difference. The don't particularity buy the analogy with __stack_pointer because SP is something that is not know at compile time so is an external dependency of the object file. The memory and table information for each object is internal, and defined internally. It also adds two extra magic symbols which I'm not sure we need (since we can just use table[0] and memory[0]). I know I should have said all that in the issue, and I'm not going to push back against landing this since it seems like a mostly internal detail.

[sunfish]

In D40875#947280, @sbc100 wrote:

Can you update the description.

Done.

Code lgtm. But I'm on the fence as to weather this change is really making any useful difference. The don't particularity buy the analogy with __stack_pointer because SP is something that is not know at compile time so is an external dependency of the object file. The memory and table information for each object is internal, and defined internally.

I don't quite understand this. The memory and table for each object is linked with those of the other objects to produce one big memory and table in the output.

It also adds two extra magic symbols which I'm not sure we need (since we can just use table[0] and memory[0]).

I agree that magic symbols are a cost. On the other hand, it doesn't feel right to hard-code index 0. I know that current wasm only supports one table and one memory, and we don't have relocations for rewriting memory indices, but it feels like those could both change in the future.

[sbc100]

Looks like this requires a corresponding lld change, broke some tests because we were looking the ->tables() on the wasm objects (which is now empty, since they are in the ->imports() now). I'm looking into a few different ways to fix.

[sbc100]

Actually if you don't mind I'd like to revert this and prepare a nicer lld-side fix that will work both before and after this change. WDYT?

[sbc100]

This can be re-landed one this lld change lands: https://reviews.llvm.org/D40989

[sbc100]

Re-landed as rL320432

llvm/trunk/lib/MC/WasmObjectWriter.cpp

void writeString(const StringRef Str) {
     writeBytes(Str);
   }
 
   void writeValueType(wasm::ValType Ty) {
     encodeSLEB128(int32_t(Ty), getStream());
   }
 
   void writeTypeSection(ArrayRef<WasmFunctionType> FunctionTypes);
-  void writeImportSection(ArrayRef<WasmImport> Imports);
+  void writeImportSection(ArrayRef<WasmImport> Imports, uint32_t DataSize,
+                          uint32_t NumElements);
   void writeFunctionSection(ArrayRef<WasmFunction> Functions);
-  void writeTableSection(uint32_t NumElements);
-  void writeMemorySection(uint32_t DataSize);
   void writeGlobalSection();
   void writeExportSection(ArrayRef<WasmExport> Exports);
   void writeElemSection(ArrayRef<uint32_t> TableElems);
   void writeCodeSection(const MCAssembler &Asm, const MCAsmLayout &Layout,
                         ArrayRef<WasmFunction> Functions);
   void writeDataSection(ArrayRef<WasmDataSegment> Segments);
   void writeNameSection(ArrayRef<WasmFunction> Functions,
                         ArrayRef<WasmImport> Imports,
for (const WasmFunctionType &FuncTy : FunctionTypes) {
     encodeULEB128(FuncTy.Returns.size(), getStream());
     for (wasm::ValType Ty : FuncTy.Returns)
       writeValueType(Ty);
   }
 
   endSection(Section);
 }
 
-void WasmObjectWriter::writeImportSection(ArrayRef<WasmImport> Imports) {
+void WasmObjectWriter::writeImportSection(ArrayRef<WasmImport> Imports,
+                                          uint32_t DataSize,
+                                          uint32_t NumElements) {
   if (Imports.empty())
     return;
 
+  uint32_t NumPages = (DataSize + wasm::WasmPageSize - 1) / wasm::WasmPageSize;
+
   SectionBookkeeping Section;
   startSection(Section, wasm::WASM_SEC_IMPORT);
 
   encodeULEB128(Imports.size(), getStream());
   for (const WasmImport &Import : Imports) {
     writeString(Import.ModuleName);
     writeString(Import.FieldName);
 
     encodeULEB128(Import.Kind, getStream());
 
     switch (Import.Kind) {
     case wasm::WASM_EXTERNAL_FUNCTION:
       encodeULEB128(Import.Type, getStream());
       break;
     case wasm::WASM_EXTERNAL_GLOBAL:
       encodeSLEB128(int32_t(Import.Type), getStream());
       encodeULEB128(int32_t(Import.IsMutable), getStream());
       break;
+    case wasm::WASM_EXTERNAL_MEMORY:
+      encodeULEB128(0, getStream()); // flags
+      encodeULEB128(NumPages, getStream()); // initial
+      break;
+    case wasm::WASM_EXTERNAL_TABLE:
+      encodeSLEB128(int32_t(Import.Type), getStream());
+      encodeULEB128(0, getStream()); // flags
+      encodeULEB128(NumElements, getStream()); // initial
+      break;
     default:
       llvm_unreachable("unsupported import kind");
     }
   }
 
   endSection(Section);
 }
 
 void WasmObjectWriter::writeFunctionSection(ArrayRef<WasmFunction> Functions) {
   if (Functions.empty())
     return;
 
   SectionBookkeeping Section;
   startSection(Section, wasm::WASM_SEC_FUNCTION);
 
   encodeULEB128(Functions.size(), getStream());
   for (const WasmFunction &Func : Functions)
     encodeULEB128(Func.Type, getStream());
 
   endSection(Section);
 }
 
-void WasmObjectWriter::writeTableSection(uint32_t NumElements) {
-  // For now, always emit the table section, since indirect calls are not
-  // valid without it. In the future, we could perhaps be more clever and omit
-  // it if there are no indirect calls.
-
-  SectionBookkeeping Section;
-  startSection(Section, wasm::WASM_SEC_TABLE);
-
-  encodeULEB128(1, getStream());                       // The number of tables.
-                                                       // Fixed to 1 for now.
-  encodeSLEB128(wasm::WASM_TYPE_ANYFUNC, getStream()); // Type of table
-  encodeULEB128(0, getStream());                       // flags
-  encodeULEB128(NumElements, getStream());             // initial
-
-  endSection(Section);
-}
-
-void WasmObjectWriter::writeMemorySection(uint32_t DataSize) {
-  // For now, always emit the memory section, since loads and stores are not
-  // valid without it. In the future, we could perhaps be more clever and omit
-  // it if there are no loads or stores.
-  SectionBookkeeping Section;
-  uint32_t NumPages = (DataSize + wasm::WasmPageSize - 1) / wasm::WasmPageSize;
-
-  startSection(Section, wasm::WASM_SEC_MEMORY);
-  encodeULEB128(1, getStream()); // number of memory spaces
-
-  encodeULEB128(0, getStream()); // flags
-  encodeULEB128(NumPages, getStream()); // initial
-
-  endSection(Section);
-}
-
 void WasmObjectWriter::writeGlobalSection() {
   if (Globals.empty())
     return;
 
   SectionBookkeeping Section;
   startSection(Section, wasm::WASM_SEC_GLOBAL);
 
   encodeULEB128(Globals.size(), getStream());
for (const uint8_t *p = (const uint8_t *)Contents.data(),
         if ((ptrdiff_t)n > end - p)
           report_fatal_error("global initial value must be valid SLEB128");
         p += n;
       }
       Globals.push_back(G);
     }
   }
 
+  // For now, always emit the memory import, since loads and stores are not
+  // valid without it. In the future, we could perhaps be more clever and omit
+  // it if there are no loads or stores.
+  MCSymbolWasm *MemorySym =
+      cast<MCSymbolWasm>(Ctx.getOrCreateSymbol("__linear_memory"));
+  WasmImport MemImport;
+  MemImport.ModuleName = MemorySym->getModuleName();
+  MemImport.FieldName = MemorySym->getName();
+  MemImport.Kind = wasm::WASM_EXTERNAL_MEMORY;
+  Imports.push_back(MemImport);
+
+  // For now, always emit the table section, since indirect calls are not
+  // valid without it. In the future, we could perhaps be more clever and omit
+  // it if there are no indirect calls.
+  MCSymbolWasm *TableSym =
+      cast<MCSymbolWasm>(Ctx.getOrCreateSymbol("__indirect_function_table"));
+  WasmImport TableImport;
+  TableImport.ModuleName = TableSym->getModuleName();
+  TableImport.FieldName = TableSym->getName();
+  TableImport.Kind = wasm::WASM_EXTERNAL_TABLE;
+  TableImport.Type = wasm::WASM_TYPE_ANYFUNC;
+  Imports.push_back(TableImport);
+
   // Populate FunctionTypeIndices and Imports.
   for (const MCSymbol &S : Asm.symbols()) {
     const auto &WS = static_cast<const MCSymbolWasm &>(S);
 
     // Register types for all functions, including those with private linkage
     // (making them
     // because wasm always needs a type signature.
     if (WS.isFunction())
for (const WasmRelocationEntry &Fixup : CodeRelocations) {
 
     registerFunctionType(*Fixup.Symbol);
   }
 
   // Write out the Wasm header.
   writeHeader(Asm);
 
   writeTypeSection(FunctionTypes);
-  writeImportSection(Imports);
+  writeImportSection(Imports, DataSize, TableElems.size());
   writeFunctionSection(Functions);
-  writeTableSection(TableElems.size());
-  writeMemorySection(DataSize);
+  // Skip the "table" section; we import the table instead.
+  // Skip the "memory" section; we import the memory instead.
   writeGlobalSection();
   writeExportSection(Exports);
   // TODO: Start Section
   writeElemSection(TableElems);
   writeCodeSection(Asm, Layout, Functions);
   writeDataSection(DataSegments);
   writeNameSection(Functions, Imports, NumFuncImports);
   writeCodeRelocSection();

llvm/trunk/test/MC/WebAssembly/external-func-address.ll

 ; CHECK-NEXT:   - Type:            TYPE
 ; CHECK-NEXT:     Signatures:      
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         ReturnType:      NORESULT
 ; CHECK-NEXT:         ParamTypes:      
 ; CHECK-NEXT:           - I32
 ; CHECK:        - Type:            IMPORT
 ; CHECK-NEXT:     Imports:
-; CHECK-NEXT:       - Module:          env
+; CHECK:            - Module:          env
+; CHECK-NEXT:         Field:           __linear_memory
+; CHECK:            - Module:          env
+; CHECK-NEXT:         Field:           __indirect_function_table
+; CHECK:            - Module:          env
 ; CHECK-NEXT:         Field:           f1
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         SigIndex:        0
 ; CHECK:        - Type:            ELEM
 ; CHECK-NEXT:     Segments:
 ; CHECK-NEXT:       - Offset:
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           0
 ; CHECK-NEXT:         Functions:       [ 0 ]
 ; CHECK:        - Type:            DATA
 ; CHECK-NEXT:     Relocations:
 ; CHECK-NEXT:       - Type:            R_WEBASSEMBLY_TABLE_INDEX_I32
 ; CHECK-NEXT:         Index:           0
 ; CHECK-NEXT:         Offset:          0x00000006

llvm/trunk/test/MC/WebAssembly/func-address.ll

 }
 
 ; CHECK:   Section {
 ; CHECK:     Type: ELEM (0x9)
 ; CHECK:     Size: 7
 ; CHECK:   }
 
 ; CHECK: Relocations [
-; CHECK:   Section (8) CODE {
+; CHECK:   Section (6) CODE {
 ; CHECK:     Relocation {
 ; CHECK:       Type: R_WEBASSEMBLY_FUNCTION_INDEX_LEB (0)
 ; CHECK:       Offset: 0x4
 ; CHECK:       Index: 0x0
 ; CHECK:     }
 ; CHECK:     Relocation {
 ; CHECK:       Type: R_WEBASSEMBLY_FUNCTION_INDEX_LEB (0)
 ; CHECK:       Offset: 0xB
 ; CHECK:       Index: 0x1
 ; CHECK:     }
 ; CHECK:     Relocation {
 ; CHECK:       Type: R_WEBASSEMBLY_TABLE_INDEX_SLEB (1)
 ; CHECK:       Offset: 0x1E
 ; CHECK:       Index: 0x0
 ; CHECK:     }
 ; CHECK:   }

llvm/trunk/test/MC/WebAssembly/init-fini-array.ll

 ; RUN: llc -mtriple wasm32-unknown-unknown-wasm -filetype=obj %s -o - | obj2yaml | FileCheck %s
 
 @global1 = global i32 1025, align 8
 
 declare void @func1()
 
 declare void @func2()
 
 @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @func1, i8* null }] 
 
 @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @func2, i8* null }]
   
 
 ; CHECK:        - Type:            IMPORT
 ; CHECK-NEXT:     Imports:         
 ; CHECK-NEXT:       - Module:          env
+; CHECK-NEXT:         Field:           __linear_memory
+; CHECK-NEXT:         Kind:            MEMORY
+; CHECK-NEXT:         Memory:
+; CHECK-NEXT:           Initial:         0x00000001
+; CHECK-NEXT:       - Module:          env
+; CHECK-NEXT:         Field:           __indirect_function_table
+; CHECK-NEXT:         Kind:            TABLE
+; CHECK-NEXT:         Table:
+; CHECK-NEXT:           ElemType:        ANYFUNC
+; CHECK-NEXT:           Limits:
+; CHECK-NEXT:             Initial:         0x00000002
+; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           func1
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         SigIndex:        0
 ; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           func2
 ; CHECK-NEXT:         Kind:            FUNCTION
 ; CHECK-NEXT:         SigIndex:        0
-; CHECK-NEXT:   - Type:            TABLE
-; CHECK-NEXT:     Tables:          
-; CHECK-NEXT:       - ElemType:        ANYFUNC
-; CHECK-NEXT:         Limits:          
-; CHECK-NEXT:           Initial:         0x00000002
-; CHECK-NEXT:   - Type:            MEMORY
-; CHECK-NEXT:     Memories:        
-; CHECK-NEXT:       - Initial:         0x00000001
 ; CHECK-NEXT:   - Type:            GLOBAL
 ; CHECK-NEXT:     Globals:         
 ; CHECK-NEXT:       - Type:            I32
 ; CHECK-NEXT:         Mutable:         false
 ; CHECK-NEXT:         InitExpr:        
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           0
 ; CHECK-NEXT:   - Type:            EXPORT

llvm/trunk/test/MC/WebAssembly/reloc-code.ll

entry:
     %tmp3 = call i32 @c()
     %tmp4 = call i32 @d()
     ret i32 %tmp2
 }
 
 
 ; CHECK: Format: WASM
 ; CHECK: Relocations [
-; CHECK-NEXT:   Section (8) CODE {
+; CHECK-NEXT:   Section (6) CODE {
 ; CHECK-NEXT:     Relocation {
 ; CHECK-NEXT:       Type: R_WEBASSEMBLY_MEMORY_ADDR_LEB (3)
 ; CHECK-NEXT:       Offset: 0x9
 ; CHECK-NEXT:       Index: 0x0
 ; CHECK-NEXT:       Addend: 0
 ; CHECK-NEXT:     }
 ; CHECK-NEXT:     Relocation {
 ; CHECK-NEXT:       Type: R_WEBASSEMBLY_MEMORY_ADDR_LEB (3)

llvm/trunk/test/MC/WebAssembly/reloc-data.ll

 ; RUN: llc -O0 -mtriple wasm32-unknown-unknown-wasm -filetype=obj %s -o - | llvm-readobj -r -expand-relocs | FileCheck %s
 
 ; foo and bar are external and internal symbols.  a and b are pointers
 ; initialized to these locations offset by 2 and -2 elements respecitively.
 @foo = external global i32, align 4
 @bar = global i64 7, align 4
 @a = global i32* getelementptr (i32, i32* @foo, i32 2), align 8
 @b = global i64* getelementptr (i64, i64* @bar, i64 -2), align 8
 @c = global [3 x i32*] [i32* @foo, i32* @foo, i32* @foo], align 16
 
 ; CHECK:      Format: WASM
 ; CHECK:      Relocations [
-; CHECK-NEXT:   Section (6) DATA {
+; CHECK-NEXT:   Section (4) DATA {
 ; CHECK-NEXT:     Relocation {
 ; CHECK-NEXT:       Type: R_WEBASSEMBLY_MEMORY_ADDR_I32 (5)
 ; CHECK-NEXT:       Offset: 0x13
 ; CHECK-NEXT:       Index: 0x0
 ; CHECK-NEXT:       Addend: 8
 ; CHECK-NEXT:     }
 ; CHECK-NEXT:     Relocation {
 ; CHECK-NEXT:       Type: R_WEBASSEMBLY_MEMORY_ADDR_I32 (5)

llvm/trunk/test/MC/WebAssembly/sections.ll

 ; CHECK:   }
 ; CHECK:  Section {
 ; CHECK:    Type: IMPORT (0x2)
 ; CHECK:  }
 ; CHECK:  Section {
 ; CHECK:    Type: FUNCTION (0x3)
 ; CHECK:  }
 ; CHECK:  Section {
-; CHECK:    Type: TABLE (0x4)
-; CHECK:  }
-; CHECK:  Section {
-; CHECK:    Type: MEMORY (0x5)
-; CHECK:    Memories [
-; CHECK:      Memory {
-; CHECK:        InitialPages: 1
-; CHECK:      }
-; CHECK:    ]
-; CHECK:  }
-; CHECK:  Section {
 ; CHECK:    Type: GLOBAL (0x6)
 ; CHECK:  }
 ; CHECK:  Section {
 ; CHECK:    Type: EXPORT (0x7)
 ; CHECK:  }
 ; CHECK:  Section {
 ; CHECK:    Type: CODE (0xA)
 ; CHECK:  }

llvm/trunk/test/MC/WebAssembly/weak-alias.ll

 }
 
 
 ; CHECK:        - Type:            TYPE
 ; CHECK-NEXT:     Signatures:      
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         ReturnType:      I32
 ; CHECK-NEXT:         ParamTypes:      
-; CHECK-NEXT:   - Type:            FUNCTION
-; CHECK-NEXT:     FunctionTypes:   [ 0, 0 ]
-; CHECK-NEXT:   - Type:            TABLE
-; CHECK-NEXT:     Tables:          
-; CHECK-NEXT:       - ElemType:        ANYFUNC
-; CHECK-NEXT:         Limits:          
-; CHECK-NEXT:           Initial:         0x00000000
-; CHECK-NEXT:   - Type:            MEMORY
-; CHECK-NEXT:     Memories:        
-; CHECK-NEXT:       - Initial:         0x00000001
+; CHECK-NEXT:   - Type:            IMPORT
+; CHECK-NEXT:     Imports:
+; CHECK-NEXT:       - Module:          env
+; CHECK-NEXT:         Field:           __linear_memory
+; CHECK-NEXT:         Kind:            MEMORY
+; CHECK-NEXT:         Memory:
+; CHECK-NEXT:           Initial:         0x00000001
+; CHECK-NEXT:       - Module:          env
+; CHECK-NEXT:         Field:           __indirect_function_table
+; CHECK-NEXT:         Kind:            TABLE
+; CHECK-NEXT:         Table:
+; CHECK-NEXT:           ElemType:        ANYFUNC
+; CHECK-NEXT:           Limits:
+; CHECK-NEXT:             Initial:         0x00000000
+; CHECK-NEXT:   - Type:            FUNCTION
+; CHECK-NEXT:     FunctionTypes:   [ 0, 0 ]
 ; CHECK-NEXT:   - Type:            GLOBAL
 ; CHECK-NEXT:     Globals:         
 ; CHECK-NEXT:       - Type:            I32
 ; CHECK-NEXT:         Mutable:         false
 ; CHECK-NEXT:         InitExpr:        
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           0
 ; CHECK-NEXT:       - Type:            I32

llvm/trunk/test/MC/WebAssembly/weak.ll

 ; RUN: llc -mtriple wasm32-unknown-unknown-wasm -filetype=obj %s -o - | obj2yaml | FileCheck %s
 
 ; Weak external data reference
 @weak_external_data = extern_weak global i32, align 4
 
 ; Weak function definition
 define weak hidden i32 @weak_function() local_unnamed_addr #0 {
 entry:
     %0 = load i32, i32* @weak_external_data, align 4
     ret i32 %0
 }
 
 ; CHECK:        - Type:            IMPORT
 ; CHECK-NEXT:     Imports:         
-; CHECK-NEXT:       - Module:          env
+; CHECK:            - Module:          env
+; CHECK-NEXT:         Field:           __linear_memory
+; CHECK:            - Module:          env
+; CHECK-NEXT:         Field:           __indirect_function_table
+; CHECK:            - Module:          env
 ; CHECK-NEXT:         Field:           weak_external_data
 ; CHECK-NEXT:         Kind:            GLOBAL
 ; CHECK-NEXT:         GlobalType:      I32
 ; CHECK-NEXT:         GlobalMutable:   false
 
 
 ; CHECK:        - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            name