[PATCH] D58315: [WebAssembly] Add .shared_memory directive

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp:163
+  Sym->setMemFlags(wasm::WASM_LIMITS_FLAG_HAS_MAX |
+                   wasm::WASM_LIMITS_FLAG_IS_SHARED);
+}
----------------
tlively wrote:
> sbc100 wrote:
> > 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.
> 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).
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).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58315/new/

https://reviews.llvm.org/D58315