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

[Thomas Lively via Phabricator via cfe-commits]

tlively added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h:40
+  // appropriate.
+  WASM_ADDRESS_SPACE_OBJECT = 1
+};
----------------
sbc100 wrote:
> tlively wrote:
> > sbc100 wrote:
> > > 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?
> > No, see my previous comment. It's more general than just globals. Tables and Memories may also reside in this address space in the future. That being said, I guess we could just name it `GLOBAL` for now and change the name once we get table and memory support.
> > 
> > Although now that I think about it, https://reviews.llvm.org/D101140 also uses this address space for locals, so it's even more general than I was thinking.
> > No, see my previous comment. It's more general than just globals. Tables and Memories may also reside in this address space in the future. That being said, I guess we could just name it `GLOBAL` for now and change the name once we get table and memory support.
> > 
> > Although now that I think about it, https://reviews.llvm.org/D101140 also uses this address space for locals, so it's even more general than I was thinking.
> 
> But are those locals backed by `global.get` and `global.set`?
> 
> OK well I don't feel so strongly about it then.  I had assumed that address space corresponded to wasm globals... 
> 
> 
No, those locals turn into normal locals backed by `local.get` and `local.set`. In that case, the address space is used in the type of `alloca` instructions.

I guess maybe OBJECT is an appropriately general label for such a general address space. Looking forward to seeing if @wingo has any better ideas!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101608