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:
> > > > 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... 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101608

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to