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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
