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


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