[PATCH] D42752: [WebAssembly] Add LLVM stub support for defined globals

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 31 13:59:14 PST 2018


sbc100 added inline comments.


================
Comment at: lib/MC/WasmObjectWriter.cpp:1213-1221
+        WasmIndex = NumGlobalImports + Globals.size();
+        WasmGlobal G;
+        G.Type = WS.getGlobalType();
+        // TODO G.InitialValue should come from somewhere, maybe the MCSymbol.
+        // This code is waiting to be used though; I expect thread-local-storage
+        // will want to declare Wasm globals.
+        G.InitialValue = 0;
----------------
ncw wrote:
> Not really needed for anything -yet  :)
> 
> I wanted to add the COMDAT_GLOBAL enum value to the header, and implement it in LLD - otherwise the LLD code just looks "lopsided". And moreover we can test it in LLD.
> 
> So having put the COMDAT_GLOBAL enum value in place, it needs supporting in obj2yaml etc... And all that code can be tested.
> 
> This chunk of code here where we handle defined global MCSymbolWasms is the only bit of truly "dead" code. It's just five lines of fairly sensible filler code, here to save somebody else some time later when they actually come to make use of global syms. If someone wants to add support in the frontend, now they won't need to worry about how to feed the globals into the object file, because it's handled here already.
> 
> We'll need a compiler intrinsic quite soon to declare/read/write globals, as something to use for implementing thread-local storage.
But didn't we just remove a bunch of code for handling globals: https://reviews.llvm.org/rL323901.

Why not wait until this is re-added to add support in the object format?  Why implement in LLD before its needed?

This is not related to the symbol table changes, right? 


Repository:
  rL LLVM

https://reviews.llvm.org/D42752





More information about the llvm-commits mailing list