sbc100 added inline comments.
================ Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1279 + if (const GlobalAddressSDNode *GA = dyn_cast<GlobalAddressSDNode>(Op)) + return WebAssembly::isManagedAddressSpace(GA->getAddressSpace()); + ---------------- sunfish wrote: > tlively wrote: > > tlively wrote: > > > Actually, should we enforce that these LLVM IR globals be thread_local, > > > since the resulting Wasm globals will be thread_local? I don't know if > > > that will affect any optimizations, but it seems like a more accurate > > > modeling. If we do that, > > > `CoalesceLocalsAndStripAtomics::stripThreadLocals` in > > > WebAssemblyTargetMachine.cpp will have to be updated to not strip the > > > thread locals corresponding to Wasm globals. > > I'd be fine handling this in a follow-up if you want to get this landed. > Note that this will be true of Worker-based threads, but not of the expected > future "wasm native threads". I wonder if this means that clang/llvm will > (eventually) need to be aware of this difference. Don't you think is very likely that even in "wasm native threads" we would want to opt into sharing like we do with wasm memories? That path would seem better for compatibility with existing worker-based threading. Obviously once we do have shared wasm globals we will need to modify llvm's assumptions, but for now I think it is a reasonable assumption/assertion that wasm globals are TLS. 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
