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

Wed May 5 15:33:46 PDT 2021

sbc100 added inline comments.

================
Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h:40
+  // appropriate.
+  WASM_ADDRESS_SPACE_OBJECT = 1
+};
----------------
tlively wrote:
> sbc100 wrote:
> > tlively wrote:
> > > sbc100 wrote:
> > > > What does "object" mean here?   Are we just talking about reference types?  Or also wasm globals that hold integers (like `__stack_pointer`).   If its just ref types that live in this address space should this be called `WASM_ADDRESS_SPACE_ANYREF`?   If its the latter should this be called `WASM_ADDRESS_SPACE_WASM_GLOBAL`?    
> > > I was also wondering about the best name here because OBJECT is somewhat vague. I think the idea is that this address space can be used for arbitrary Wasm globals of any type, but it could also be used later for things like additional tables and memories. It's unclear whether those would need separate address spaces for some reason, but if they don't, re-using this address space 1 would be best.
> > > 
> > > Maybe `WASM_ADDRESS_SPACE_STATIC` would be a better name because it will be used for things that are given static indices in the final module?
> > > I was also wondering about the best name here because OBJECT is somewhat vague. I think the idea is that this address space can be used for arbitrary Wasm globals of any type, but it could also be used later for things like additional tables and memories. It's unclear whether those would need separate address spaces for some reason, but if they don't, re-using this address space 1 would be best.
> > > 
> > > Maybe `WASM_ADDRESS_SPACE_STATIC` would be a better name because it will be used for things that are given static indices in the final module?
> > 
> > But that is also true for static data symbols that point memory addresess.
> Hmm, maybe `WASM_ADDRESS_SPACE_MODULE_ELEMENT`?
The address space is the space of wasm globals right?  So I feel like it should probably contain the word global... shouldn't it?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101608