[WebAssembly] Add LLVM stub support for defined globals

[ncw]

Summary

It's not much use yet, but getting the constants into the headers and adding some stub support enables at least LLD to be tested against defined & undefined globals.

Comes after D42495. Doesn't break LLD.

[sbc100]

What is this needed for? Didn't we just drop support for emitting wasm globals?

[ncw]

lib/MC/WasmObjectWriter.cpp

1213–1221

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.

[sbc100]

lib/MC/WasmObjectWriter.cpp

1213–1221

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?

[ncw]

lib/MC/WasmObjectWriter.cpp

1213–1221

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?

[ncw]

Abandoning; COMDAT for globals would be nice (for completeness) but isn't important now - no need to fight for it currently.

I still think it should probably be added to Linking.md before the first formally-complete release, but I agree it's not important.

include/llvm/BinaryFormat/Wasm.h

struct WasmGlobalType {
   int32_t Type;
   bool Mutable;
 };
 
 struct WasmGlobal {
   uint32_t Index;
   WasmGlobalType Type;
   WasmInitExpr InitExpr;
+  StringRef Comdat; // from the "comdat info" section
 };
 
 struct WasmImport {
   StringRef Module;
   StringRef Field;
   uint32_t Kind;
   union {
     uint32_t SigIndex;
enum : unsigned {
   WASM_COMDAT_INFO    = 0x7,
   WASM_SYMBOL_TABLE   = 0x8,
 };
 
 // Kind codes used in the custom "linking" section in the WASM_COMDAT_INFO
 enum : unsigned {
   WASM_COMDAT_DATA        = 0x0,
   WASM_COMDAT_FUNCTION    = 0x1,
+  WASM_COMDAT_GLOBAL      = 0x2,
 };
 
 // Kind codes used in the custom "linking" section in the WASM_SYMBOL_TABLE
 enum : unsigned {
   WASM_SYMTAB_FUNCTION = 0x0,
   WASM_SYMTAB_DATA     = 0x1,
   WASM_SYMTAB_GLOBAL   = 0x2,
 };

lib/MC/WasmObjectWriter.cpp

       }
       DEBUG(dbgs() << "  -> segment index: "
                    << (SegInfo.hasValue() ? (int)SegInfo.getValue().Segment : -1)
                    << "\n");
     } else {
       // A "true" Wasm global (currently just __stack_pointer)
       unsigned WasmIndex;
       if (WS.isDefined()) {
-        report_fatal_error("don't yet support defined globals");
+        // A definition. Write out the Global definition.
+        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;
+        WasmIndices[&WS] = WasmIndex;
+        Globals.push_back(G);
+
+        auto &Section = static_cast<MCSectionWasm &>(WS.getSection());
+        if (const MCSymbolWasm *C = Section.getGroup()) {
+          Comdats[C->getName()].emplace_back(
+              WasmComdatEntry{wasm::WASM_COMDAT_GLOBAL, WasmIndex});
+        }
       } else {
         // An import; the index was assigned above
         WasmIndex = WasmIndices.find(&WS)->second;
       }
 
       DEBUG(dbgs() << "  -> global index: " << WasmIndex << "\n");
     }
 

lib/Object/WasmObjectFile.cpp

while (EntryCount--) {
         if (!isDefinedFunctionIndex(Index))
           return make_error<GenericBinaryError>("COMDAT function index out of range",
                                                 object_error::parse_failed);
         if (!getDefinedFunction(Index).Comdat.empty())
           return make_error<GenericBinaryError>("Function in two COMDATs",
                                                 object_error::parse_failed);
         getDefinedFunction(Index).Comdat = Name;
         break;
+      case wasm::WASM_COMDAT_GLOBAL:
+        if (!isDefinedGlobalIndex(Index))
+          return make_error<GenericBinaryError>("COMDAT global index out of range",
+                                                object_error::parse_failed);
+        if (!getDefinedGlobal(Index).Comdat.empty())
+          return make_error<GenericBinaryError>("Global in two COMDATs",
+                                                object_error::parse_failed);
+        getDefinedGlobal(Index).Comdat = Name;
+        break;
       }
     }
   }
   return Error::success();
 }
 
 WasmSection* WasmObjectFile::findCustomSectionByName(StringRef Name) {
   for (WasmSection& Section : Sections) {

lib/ObjectYAML/WasmYAML.cpp

void MappingTraits<WasmYAML::InitFunction>::mapping(
   IO.mapRequired("FunctionIndex", Init.FunctionIndex);
 }
 
 void ScalarEnumerationTraits<WasmYAML::ComdatKind>::enumeration(
     IO &IO, WasmYAML::ComdatKind &Kind) {
 #define ECase(X) IO.enumCase(Kind, #X, wasm::WASM_COMDAT_##X);
   ECase(FUNCTION);
   ECase(DATA);
+  ECase(GLOBAL);
 #undef ECase
 }
 
 void MappingTraits<WasmYAML::ComdatEntry>::mapping(
     IO &IO, WasmYAML::ComdatEntry &ComdatEntry) {
   IO.mapRequired("Kind", ComdatEntry.Kind);
   IO.mapRequired("Index", ComdatEntry.Index);
 }

tools/obj2yaml/wasm2yaml.cpp

if (WasmSec.Name == "name") {
     }
     for (auto &Func : Obj.functions()) {
       if (!Func.Comdat.empty()) {
         auto &Comdat = LinkingSec->Comdats[ComdatIndexes[Func.Comdat]];
         Comdat.Entries.emplace_back(
                 WasmYAML::ComdatEntry{wasm::WASM_COMDAT_FUNCTION, Func.Index});
       }
     }
+    for (auto &G : Obj.globals()) {
+      if (!G.Comdat.empty()) {
+        auto &Comdat = LinkingSec->Comdats[ComdatIndexes[G.Comdat]];
+        Comdat.Entries.emplace_back(
+                WasmYAML::ComdatEntry{wasm::WASM_COMDAT_GLOBAL, G.Index});
+      }
+    }
     uint32_t SegmentIndex = 0;
     for (const object::WasmSegment &Segment : Obj.dataSegments()) {
       if (!Segment.Data.Name.empty()) {
         WasmYAML::SegmentInfo SegmentInfo;
         SegmentInfo.Name = Segment.Data.Name;
         SegmentInfo.Index = SegmentIndex;
         SegmentInfo.Alignment = Segment.Data.Alignment;
         SegmentInfo.Flags = Segment.Data.Flags;