wingo added inline comments.
================ Comment at: llvm/lib/Target/WebAssembly/Utils/WebAssemblyUtilities.h:39 + // pointers are lowered to global.get / global.set or local.get / local.set, + // as appropriate. + WASM_ADDRESS_SPACE_MANAGED = 1 ---------------- sunfish wrote: > tlively wrote: > > sunfish wrote: > > > tlively wrote: > > > > sunfish wrote: > > > > > Sorry to throw more paint at the bikeshed here, but as someone who's > > > > > only following along at a high-level here, I found it confusing > > > > > whether this is talking about the wasm globals themselves, or the > > > > > objects referred to by reference values in the wasm globals. I think > > > > > the feature here is talking about the wasm globals themselves, but > > > > > "managed" initially made me think it might be talking about the > > > > > objects they reference, which in a browser context especially are > > > > > "managed" in every sense of the word. > > > > Fair point. What does everyone think about `INDEXED`, because it is > > > > used to represent objects given static indexes (in the WebAssembly > > > > sense) in the final binary. > > > Do I understand correctly that global variables and local variables are > > > being assigned addresses within the same conceptual address space here? > > > > > > How about `WASM_VARIABLES` or `WASM_VARS`? The wasm spec terms for these > > > are global variables and local variables. > > > > > > > > Sure, that works for me. We may want to rename it in the future if we end > > up using the same address space for tables and memories, but we can cross > > that bridge when we get there. > Tables and memories have their own index spaces in wasm, so it seems like > they'd want their own address spaces here, perhaps 2 and 3, respectively? I > also agree that we can figure this out later. `WASM_VARS` is fine with me, with one caveat -- in https://reviews.llvm.org/D101140 where we add support for locals, we add a new "core" TargetStackID enum which describes static `alloca` that is actually a wasm local. Really this enum describes objects allocated on a parallel stack, outside of linear memory. It was nice to use the same name to describe the address space and the stack id, but I don't think `variable` or `wasm-var` are good core enum names. It feels like there is a core concept here for systems that have two stacks -- one of named objects and one of linear-memory bytes. I thought `object` or `managed` could be a good name for the former, but evidently not :) I will let this one sit overnight; if there are no other good ideas before tomorrow I will do `WASM_ADDRESS_SPACE_WASM_VARS` and either find a new name for `TargetStackID` or attempt to use a target-specific definition. 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
