[WebAssembly] Add .shared_memory directive

[tlively]

Summary

For object files, emitting the .shared_memory directive sets the
shared bit on the __linear_memory import symbol, so memory flags are
also added to wasm symbols. The directive is emitted in the AsmPrinter
if the thread model is multithreaded to communicate to the linker that
this object is built for a multithreaded context. Depends on D57938.

[sbc100]

llvm/include/llvm/MC/MCSymbolWasm.h

26

Seems strange to attach this to a symol.

llvm/test/MC/WebAssembly/assembler-binary.ll

25

This looks like its starting a new section. Could we call this ".shared_memory"? Or is there some other place in the asm syntax there this attribute might be specified?

Actually do we need this to be in the .s format at all? seems like it could be specified as an assembler flag? Like the march/mcpu stuff.. that doesn't live in the .s format right?

llvm/test/MC/WebAssembly/event-section.ll

1–2

Can we make this the default instead?

[tlively]

llvm/include/llvm/MC/MCSymbolWasm.h

26

Different kinds of symbols already store their special information in this object, such as their Signatures or Types. I think it makes sense for memory imports to do the same.

llvm/test/MC/WebAssembly/assembler-binary.ll

25

Yes, calling it ".shared_memory" would be fine, although it is really meant to communicate that the entire module was produced to be thread-aware, not just that the memory is shared. That this corresponds to the shared bit being set in the object file is mostly an implementation detail. I'm open to suggestions for other places this information might live, but up at the top of the file seemed appropriate because it applies to the entire module.

I believe the march/mcpu options are not recorded in the .s format because they only affect codegen, which has already taken place by the time you have an assembly file. In contrast, for webassembly the information that code was produced by a thread-aware toolchain needs to be propagated through to the linker alongside that code, whether or not there is a separate assembly step in between. There is no way to propagate this information through an intermediate assembly step except by assembler directive.

llvm/test/MC/WebAssembly/event-section.ll

1–2

I'm not sure we can. The option and its default value are are statically defined in include/llvm/CodeGen/CommandFlags.inc, and I don't think it's possible to make the default value of an option like that depend on the value of another option.

[tlively]

@sbc100 I'm looking at the linker to figure out how to implement shared memory-aware linking, and it looks like the linker doesn't even look in the imports section of the wobject. Why even bother importing __linear_memory in object files at all? Is it just to make the object file a spec-compliant wasm? I think I'm going to have to make the linker look at the import section for the sole purpose of fetching out this shared bit on the memory import. This isn't a problem, but it did surprise me.

[sbc100]

In D58315#1400286, @tlively wrote:

@sbc100 I'm looking at the linker to figure out how to implement shared memory-aware linking, and it looks like the linker doesn't even look in the imports section of the wobject. Why even bother importing __linear_memory in object files at all? Is it just to make the object file a spec-compliant wasm? I think I'm going to have to make the linker look at the import section for the sole purpose of fetching out this shared bit on the memory import. This isn't a problem, but it did surprise me.

Yes, its purely to make the object file valid.

I'm tempted to have the linker driven by a flag for this rather then driven by the input file.

If you were going to check that kind of think I would do it when cheking the format of the object file. Perhaps is lld::wasm::createObjectFile.

[aardappel]

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

654

This applies to the most recent symbol? Seems many directives we have explicitly refer to the symbol they're changing, like .functype, .eventtype, .globaltype, so would it make sense for this to that too?

Or is there only ever one such flag and one such symbol, ever?

[tlively]

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

654

There is meant to only ever be one such flag for the entire module, and the only memory import is the __linear_memory symbol that this affects. Currently the ObjectFileWriter unconditionally emits __linear_memory, and this only modifies the flags it is emitted with.

[sbc100]

llvm/lib/MC/WasmObjectWriter.cpp

756

I guess this makes sense for completeness, but an object file really doesn't make sense to have a max for its memory.

[tlively]

llvm/lib/MC/WasmObjectWriter.cpp

756

Yeah, unfortunately it is necessary if we want object files to validate. Also unfortunately, it is invalid to have the limits be shared but not have a max, so we have to just come up with some arbitrary max.

[tlively]

• Rename .shared to .shared_memory, add comment

[sbc100]

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

163

It still seems wrong to attach this information to a symbol, is there not some module level state we can attach it too?

I don't think __linear_memory should be and MCSymbol at all. We don't have a symbol type for a memory. It looks like it is referenced in WasmObjectWriter.cpp but I can't see why it would be.

[tlively]

An alternative to the approach taken in this CL that explicitly records that the object is thread-aware in a new metadata section is presented here: https://github.com/WebAssembly/tool-conventions/pull/99

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

163

Changing approaches to use a target features section would make this change to MCSymbol unnecessary (although WasmObjectWriter would still treat __linear_memory as a symbol).

[sbc100]

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

163

Not if I can land this first :) https://reviews.llvm.org/D58487

Seriously though, is there some other place we can attach the "shared" attribute so that it can later be read by the object writer? I don't think we need to block on the target features section.. I'm still happy with the flag on the memory import, just that we set that flag based on something other than an MCSymbol? (Not sure how reasonable this request is).

[tlively]

It looks like we have consensus that a target features section would be a good idea, so I'm closing out this CL.

llvm/include/llvm/BinaryFormat/Wasm.h

enum : unsigned {
   WASM_OPCODE_END = 0x0b,
   WASM_OPCODE_GLOBAL_GET = 0x23,
   WASM_OPCODE_I32_CONST = 0x41,
   WASM_OPCODE_I64_CONST = 0x42,
   WASM_OPCODE_F32_CONST = 0x43,
   WASM_OPCODE_F64_CONST = 0x44,
 };
 
-enum : unsigned {
+enum : uint8_t {
   WASM_LIMITS_FLAG_HAS_MAX = 0x1,
   WASM_LIMITS_FLAG_IS_SHARED = 0x2,
 };
 
 enum : unsigned {
   WASM_SEGMENT_IS_PASSIVE = 0x01,
   WASM_SEGMENT_HAS_MEMINDEX = 0x02,
 };

llvm/include/llvm/MC/MCSymbolWasm.h

class MCSymbolWasm : public MCSymbol {
   bool IsWeak = false;
   bool IsHidden = false;
   bool IsComdat = false;
   Optional<std::string> ImportModule;
   Optional<std::string> ImportName;
   wasm::WasmSignature *Signature = nullptr;
   Optional<wasm::WasmGlobalType> GlobalType;
   Optional<wasm::WasmEventType> EventType;
+  uint8_t MemFlags = 0;
 
   /// 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:
   // Use a module name of "env" for now, for compatibility with existing tools.
   // This is temporary, and may change, as the ABI is not yet stable.
public:
   const StringRef getImportName() const {
       if (ImportName.hasValue()) {
           return ImportName.getValue();
       }
       return getName();
   }
   void setImportName(StringRef Name) { ImportName = Name; }
 
+  uint8_t getMemFlags() const { return MemFlags; }
+  void setMemFlags(uint8_t F) { MemFlags = F; }
+
   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; }

llvm/lib/MC/WasmObjectWriter.cpp

for (const wasm::WasmImport &Import : Imports) {
     case wasm::WASM_EXTERNAL_FUNCTION:
       encodeULEB128(Import.SigIndex, W.OS);
       break;
     case wasm::WASM_EXTERNAL_GLOBAL:
       W.OS << char(Import.Global.Type);
       W.OS << char(Import.Global.Mutable ? 1 : 0);
       break;
     case wasm::WASM_EXTERNAL_MEMORY:
-      encodeULEB128(0, W.OS);        // flags
-      encodeULEB128(NumPages, W.OS); // initial
+      encodeULEB128(Import.Memory.Flags, W.OS); // flags
+      encodeULEB128(NumPages, W.OS);            // initial
+      if (Import.Memory.Flags & wasm::WASM_LIMITS_FLAG_HAS_MAX)
+        encodeULEB128(NumPages, W.OS);          // max
       break;
     case wasm::WASM_EXTERNAL_TABLE:
       W.OS << char(Import.Table.ElemType);
       encodeULEB128(0, W.OS);           // flags
       encodeULEB128(NumElements, W.OS); // initial
       break;
     case wasm::WASM_EXTERNAL_EVENT:
       encodeULEB128(Import.Event.Attribute, W.OS);
uint64_t WasmObjectWriter::writeObject(MCAssembler &Asm,
   // valid without it. In the future, we could perhaps be more clever and omit
   // it if there are no loads or stores.
   auto *MemorySym =
       cast<MCSymbolWasm>(Ctx.getOrCreateSymbol("__linear_memory"));
   wasm::WasmImport MemImport;
   MemImport.Module = MemorySym->getImportModule();
   MemImport.Field = MemorySym->getImportName();
   MemImport.Kind = wasm::WASM_EXTERNAL_MEMORY;
+  MemImport.Memory.Flags = MemorySym->getMemFlags();
   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.
   auto *TableSym =
       cast<MCSymbolWasm>(Ctx.getOrCreateSymbol("__indirect_function_table"));
   wasm::WasmImport TableImport;

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

if (DirectiveID.getString() == ".local") {
       SmallVector<wasm::ValType, 4> Locals;
       if (parseRegTypeList(Locals))
         return true;
       TOut.emitLocal(Locals);
       CurrentState = FunctionLocals;
       return expect(AsmToken::EndOfStatement, "EOL");
     }
 
+    if (DirectiveID.getString() == ".shared_memory") {
+      TOut.emitSharedMemory();
+      return expect(AsmToken::EndOfStatement, "EOL");
+    }
+
     return true; // We didn't process this directive.
   }
 
   bool MatchAndEmitInstruction(SMLoc IDLoc, unsigned & /*Opcode*/,
                                OperandVector &Operands, MCStreamer &Out,
                                uint64_t &ErrorInfo,
                                bool MatchingInlineAsm) override {
     MCInst Inst;

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

public:
   virtual void emitEventType(const MCSymbolWasm *Sym) = 0;
   /// .import_module
   virtual void emitImportModule(const MCSymbolWasm *Sym,
                                 StringRef ImportModule) = 0;
   /// .import_name
   virtual void emitImportName(const MCSymbolWasm *Sym,
                               StringRef ImportName) = 0;
 
+  /// .shared_memory
+  virtual void emitSharedMemory() = 0;
+
 protected:
   void emitValueType(wasm::ValType Type);
 };
 
 /// This part is for ascii assembly output
 class WebAssemblyTargetAsmStreamer final : public WebAssemblyTargetStreamer {
   formatted_raw_ostream &OS;
   void emitSignature(const wasm::WasmSignature *Sig);
   void emitParamList(const wasm::WasmSignature *Sig);
   void emitReturnList(const wasm::WasmSignature *Sig);
 
 public:
   WebAssemblyTargetAsmStreamer(MCStreamer &S, formatted_raw_ostream &OS);
 
   void emitLocal(ArrayRef<wasm::ValType> Types) override;
   void emitEndFunc() override;
   void emitFunctionType(const MCSymbolWasm *Sym) override;
   void emitIndIdx(const MCExpr *Value) override;
   void emitGlobalType(const MCSymbolWasm *Sym) override;
   void emitEventType(const MCSymbolWasm *Sym) override;
   void emitImportModule(const MCSymbolWasm *Sym, StringRef ImportModule) override;
   void emitImportName(const MCSymbolWasm *Sym, StringRef ImportName) override;
+  void emitSharedMemory() override;
 };
 
 /// This part is for Wasm object output
 class WebAssemblyTargetWasmStreamer final : public WebAssemblyTargetStreamer {
 public:
   explicit WebAssemblyTargetWasmStreamer(MCStreamer &S);
 
   void emitLocal(ArrayRef<wasm::ValType> Types) override;
   void emitEndFunc() override;
   void emitFunctionType(const MCSymbolWasm *Sym) override {}
   void emitIndIdx(const MCExpr *Value) override;
   void emitGlobalType(const MCSymbolWasm *Sym) override {}
   void emitEventType(const MCSymbolWasm *Sym) override {}
   void emitImportModule(const MCSymbolWasm *Sym,
                         StringRef ImportModule) override {}
   void emitImportName(const MCSymbolWasm *Sym,
                       StringRef ImportName) override {}
+  void emitSharedMemory() override;
 };
 
 /// This part is for null output
 class WebAssemblyTargetNullStreamer final : public WebAssemblyTargetStreamer {
 public:
   explicit WebAssemblyTargetNullStreamer(MCStreamer &S)
       : WebAssemblyTargetStreamer(S) {}
 
   void emitLocal(ArrayRef<wasm::ValType>) override {}
   void emitEndFunc() override {}
   void emitFunctionType(const MCSymbolWasm *) override {}
   void emitIndIdx(const MCExpr *) override {}
   void emitGlobalType(const MCSymbolWasm *) override {}
   void emitEventType(const MCSymbolWasm *) override {}
   void emitImportModule(const MCSymbolWasm *, StringRef) override {}
   void emitImportName(const MCSymbolWasm *, StringRef) override {}
+  void emitSharedMemory() override {}
 };
 
 } // end namespace llvm
 
 #endif

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

 }
 
 void WebAssemblyTargetAsmStreamer::emitImportName(const MCSymbolWasm *Sym,
                                                   StringRef ImportName) {
   OS << "\t.import_name\t" << Sym->getName() << ", "
                            << ImportName << '\n';
 }
 
+void WebAssemblyTargetAsmStreamer::emitSharedMemory() {
+  OS << "\t.shared_memory\n";
+}
+
 void WebAssemblyTargetAsmStreamer::emitIndIdx(const MCExpr *Value) {
   OS << "\t.indidx  \t" << *Value << '\n';
 }
 
 void WebAssemblyTargetWasmStreamer::emitLocal(ArrayRef<wasm::ValType> Types) {
   SmallVector<std::pair<wasm::ValType, uint32_t>, 4> Grouped;
   for (auto Type : Types) {
     if (Grouped.empty() || Grouped.back().first != Type)

 
 void WebAssemblyTargetWasmStreamer::emitEndFunc() {
   llvm_unreachable(".end_func is not needed for direct wasm output");
 }
 
 void WebAssemblyTargetWasmStreamer::emitIndIdx(const MCExpr *Value) {
   llvm_unreachable(".indidx encoding not yet implemented");
 }
+
+void WebAssemblyTargetWasmStreamer::emitSharedMemory() {
+  // __linear_memory is always included in the object, this just makes it shared
+  auto Sym = cast<MCSymbolWasm>(
+      getStreamer().getContext().getOrCreateSymbol("__linear_memory"));
+  Sym->setMemFlags(wasm::WASM_LIMITS_FLAG_HAS_MAX |
+                   wasm::WASM_LIMITS_FLAG_IS_SHARED);
+}

llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.h

bool runOnMachineFunction(MachineFunction &MF) override {
     MFI = MF.getInfo<WebAssemblyFunctionInfo>();
     return AsmPrinter::runOnMachineFunction(MF);
   }
 
   //===------------------------------------------------------------------===//
   // AsmPrinter Implementation.
   //===------------------------------------------------------------------===//
 
+  void EmitStartOfAsmFile(Module &M) override;
   void EmitEndOfAsmFile(Module &M) override;
   void EmitProducerInfo(Module &M);
   void EmitJumpTableInfo() override;
   void EmitConstantPool() override;
   void EmitFunctionBodyStart() override;
   void EmitInstruction(const MachineInstr *MI) override;
   const MCExpr *lowerConstant(const Constant *CV) override;
   bool PrintAsmOperand(const MachineInstr *MI, unsigned OpNo,

llvm/lib/Target/WebAssembly/WebAssemblyAsmPrinter.cpp

WebAssemblyTargetStreamer *WebAssemblyAsmPrinter::getTargetStreamer() {
   MCTargetStreamer *TS = OutStreamer->getTargetStreamer();
   return static_cast<WebAssemblyTargetStreamer *>(TS);
 }
 
 //===----------------------------------------------------------------------===//
 // WebAssemblyAsmPrinter Implementation.
 //===----------------------------------------------------------------------===//
 
+void WebAssemblyAsmPrinter::EmitStartOfAsmFile(Module &M) {
+  if (TM.Options.ThreadModel != ThreadModel::Single) {
+    getTargetStreamer()->emitSharedMemory();
+  }
+}
+
 void WebAssemblyAsmPrinter::EmitEndOfAsmFile(Module &M) {
   for (auto &It : OutContext.getSymbols()) {
     // Emit a .globaltype and .eventtype declaration.
     auto Sym = cast<MCSymbolWasm>(It.getValue());
     if (Sym->getType() == wasm::WASM_SYMBOL_TYPE_GLOBAL)
       getTargetStreamer()->emitGlobalType(Sym);
     else if (Sym->getType() == wasm::WASM_SYMBOL_TYPE_EVENT)
       getTargetStreamer()->emitEventType(Sym);

llvm/test/MC/WebAssembly/assembler-binary.ll

 entry:
   call void @bar()
   ret void
 }
 
 ; Checking assembly is not the point of this test, but if something breaks
 ; it is easier to spot it here than in the yaml output.
 
-; ASM:      .text
-; ASM:        .file "assembler-binary.ll"
-; ASM:        .globl  foo
-; ASM:      foo:
-; ASM-NEXT:   .functype foo (i32) -> ()
-; ASM-NEXT:   call  bar@FUNCTION
-; ASM-NEXT:   end_function
-; ASM:        .functype bar () -> ()
+; ASM:          .text
+; ASM:          .shared
+; ASM-NEXT:     .file "assembler-binary.ll"
+; ASM-DAG:     .section .text.foo,"",@
+; ASM-DAG:     .globl foo
+
+; ASM:     foo:
+; ASM-NEXT:     .functype foo (i32) -> ()
+; ASM-NEXT:     call bar@FUNCTION
+; ASM-NEXT:     end_function
+; ASM:         .functype bar () -> ()
 
 
 ; CHECK:      --- !WASM
 ; CHECK-NEXT: FileHeader:
 ; CHECK-NEXT:   Version:         0x00000001
 ; CHECK-NEXT: Sections:
 ; CHECK-NEXT:   - Type:            TYPE
 ; CHECK-NEXT:     Signatures:
 ; CHECK-NEXT:       - Index:           0
 ; CHECK-NEXT:         ReturnType:      NORESULT
 ; CHECK-NEXT:         ParamTypes:
 ; CHECK-NEXT:           - I32
 ; CHECK-NEXT:       - Index:           1
 ; CHECK-NEXT:         ReturnType:      NORESULT
 ; CHECK-NEXT:         ParamTypes:      []
 ; 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:           Flags:           [ HAS_MAX, IS_SHARED ]
 ; CHECK-NEXT:           Initial:         0x00000000
+; CHECK-NEXT:           Maximum:         0x00000000
 ; CHECK-NEXT:       - Module:          env
 ; CHECK-NEXT:         Field:           __indirect_function_table
 ; CHECK-NEXT:         Kind:            TABLE
 ; CHECK-NEXT:         Table:
 ; CHECK-NEXT:           ElemType:        FUNCREF
 ; CHECK-NEXT:           Limits:
 ; CHECK-NEXT:             Initial:         0x00000000
 ; CHECK-NEXT:       - Module:          env

llvm/test/MC/WebAssembly/custom-sections.ll

-; RUN: llc -filetype=obj %s -o - | llvm-readobj -s | FileCheck %s
+; RUN: llc -filetype=obj -thread-model=single %s -o - | llvm-readobj -s | FileCheck %s
 
 ; Test the mechanism for defining user custom sections.
 
 target triple = "wasm32-unknown-unknown"
 
 !0 = !{ !"red", !"foo" }
 !1 = !{ !"green", !"bar" }
 !2 = !{ !"green", !"qux" }

llvm/test/MC/WebAssembly/event-section.ll

-; RUN: llc -filetype=obj -exception-model=wasm -mattr=+exception-handling %s -o - | obj2yaml | FileCheck %s
-; RUN: llc -filetype=obj -exception-model=wasm -mattr=+exception-handling %s -o - | llvm-readobj -s | FileCheck -check-prefix=SEC %s
+; RUN: llc -filetype=obj -thread-model=single -exception-model=wasm -mattr=+exception-handling %s -o - | obj2yaml | FileCheck %s
+; RUN: llc -filetype=obj -thread-model=single -exception-model=wasm -mattr=+exception-handling %s -o - | llvm-readobj -s | FileCheck -check-prefix=SEC %s
 
 target triple = "wasm32-unknown-unknown"
 
 declare void @llvm.wasm.throw(i32, i8*)
 
 define i32 @test_throw0(i8* %p) {
   call void @llvm.wasm.throw(i32 0, i8* %p)
   ret i32 0

llvm/test/ObjectYAML/wasm/memory_section.yaml

 # RUN: yaml2obj %s | obj2yaml | FileCheck %s
 --- !WASM
 FileHeader:
   Version:         0x00000001
 Sections:
   - Type:            MEMORY
     Memories:         
       - Flags:           [ HAS_MAX ]
         Initial:         0x00000002
         Maximum:         0x000000FF
       - Initial:         0x00000003
+      - Flags:           [ HAS_MAX, IS_SHARED ]
+        Initial:         0x00000042
+        Maximum:         0x00000042
+
 ...
 # CHECK: --- !WASM
 # CHECK: FileHeader:
 # CHECK:   Version:           0x00000001
 # CHECK: Sections:
 # CHECK:   - Type:            MEMORY
 # CHECK:     Memories:         
 # CHECK:       - Flags:            [ HAS_MAX ]
 # CHECK:         Initial:          0x00000002
 # CHECK:         Maximum:          0x000000FF
 # CHECK:       - Initial:          0x00000003
+# CHECK:       - Flags:            [ HAS_MAX, IS_SHARED ]
+# CHECK:         Initial:          0x00000042
+# CHECK:         Maximum:          0x00000042
 # CHECK: ...