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

[Nicholas Wilson via Phabricator via llvm-commits]

ncw 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;
----------------
sbc100 wrote:
> 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? 
This change doesn't need to go in, you're right. It can be left until needed, that's fine.

It's not necessary for the symbol table stuff. However, it does affect the object format (new COMDAT_GLOBAL flag) so from the point of view of stabilising the object format I felt it did make sense to do now.

Then the linking conventions could be frozen, and LLD tested, even though the frontend doesn't yet use it.

I'm starting to think about the frontend - what would brilliant in my opinion would be either a new attribute like `__attribute__((wasm_global))` or an existing one like `__thread` allowing you to make declarations like:

```
long __thread thread_base = 0;
```

The attribute would make the variable into a global (only allowed on i32/i64/f32/f64 variables at global scope), and the initialiser's value would propagate through to be set on the Wasm global as its initial value.

Do you know if anyone's planning to implement thread-locals using an attribute like that?


Repository:
  rL LLVM

https://reviews.llvm.org/D42752