[PATCH] D101608: [WebAssembly] Support for WebAssembly globals in LLVM IR

[Dan Gohman via Phabricator via llvm-commits]

sunfish added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h:39
+  // pointers are lowered to global.get / global.set or local.get / local.set,
+  // as appropriate.
+  WASM_ADDRESS_SPACE_MANAGED = 1
----------------
tlively wrote:
> sunfish wrote:
> > tlively wrote:
> > > sunfish wrote:
> > > > Sorry to throw more paint at the bikeshed here, but as someone who's only following along at a high-level here, I found it confusing whether this is talking about the wasm globals themselves, or the objects referred to by reference values in the wasm globals. I think the feature here is talking about the wasm globals themselves, but "managed" initially made me think it might be talking about the objects they reference, which in a browser context especially are "managed" in every sense of the word.
> > > Fair point. What does everyone think about `INDEXED`, because it is used to represent objects given static indexes (in the WebAssembly sense) in the final binary.
> > Do I understand correctly that global variables and local variables are being assigned addresses within the same conceptual address space here?
> > 
> > How about `WASM_VARIABLES` or `WASM_VARS`? The wasm spec terms for these are global variables and local variables.
> > 
> > 
> Sure, that works for me. We may want to rename it in the future if we end up using the same address space for tables and memories, but we can cross that bridge when we get there.
Tables and memories have their own index spaces in wasm, so it seems like they'd want their own address spaces here, perhaps 2 and 3, respectively? I also agree that we can figure this out later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101608