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

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

Reply via email to