[WebAssembly] Pre-symtab tidy of LLVM globals. NFC.

[ncw]

Summary

• Remove dead code for handling the .global_variables section in WasmObjectWriter, which is unused and won't work anyway because it doesn't generate relocations for the variables
• Factored out common struct WasmGlobalType, which matches the name for that tuple in the Wasm spec
• Renamed some methods to "isBindingGlobal", "isTypeGlobal" to avoid ambiguity

[sbc100]

lib/MC/WasmObjectWriter.cpp

728 ↗

(On Diff #132212)

I would revert to encodeSLEB128, not that its just a uint32_t.

1141 ↗

(On Diff #132212)

I would change PtrType .. since both uses now have a type cast.

[sbc100]

LGTM with a couple of comment.

I guess we can bring back the global handling later, but for now its essentially dead code.

llvm/trunk/include/llvm/BinaryFormat/Wasm.h

union {
     int32_t Int32;
     int64_t Int64;
     int32_t Float32;
     int64_t Float64;
     uint32_t Global;
   } Value;
 };
 
-struct WasmGlobal {
-  uint32_t Index;
+struct WasmGlobalType {
   int32_t Type;
   bool Mutable;
+};
+
+struct WasmGlobal {
+  uint32_t Index;
+  WasmGlobalType Type;
   WasmInitExpr InitExpr;
 };
 
 struct WasmImport {
   StringRef Module;
   StringRef Field;
   uint32_t Kind;
   union {
     uint32_t SigIndex;
-    WasmGlobal Global;
+    WasmGlobalType Global;
     WasmTable Table;
     WasmLimits Memory;
   };
 };
 
 struct WasmLocalDecl {
   int32_t Type;
   uint32_t Count;

llvm/trunk/include/llvm/Object/Wasm.h

public:
   uint32_t AltIndex = 0;
   bool HasAltIndex = false;
 
   void setAltIndex(uint32_t Index) {
     HasAltIndex = true;
     AltIndex = Index;
   }
 
-  bool isFunction() const {
+  bool isTypeFunction() const {
     return Type == WasmSymbol::SymbolType::FUNCTION_IMPORT ||
            Type == WasmSymbol::SymbolType::FUNCTION_EXPORT;
   }
 
+  bool isTypeGlobal() const {
+    return Type == SymbolType::GLOBAL_IMPORT ||
+           Type == SymbolType::GLOBAL_EXPORT;
+  }
+
 
-  bool isWeak() const {
+  bool isBindingWeak() const {
     return getBinding() == wasm::WASM_SYMBOL_BINDING_WEAK;
   }
 
-  bool isGlobal() const {
+  bool isBindingGlobal() const {
     return getBinding() == wasm::WASM_SYMBOL_BINDING_GLOBAL;
   }
 
-  bool isLocal() const {
+  bool isBindingLocal() const {
     return getBinding() == wasm::WASM_SYMBOL_BINDING_LOCAL;
   }
 
   unsigned getBinding() const {
     return Flags & wasm::WASM_SYMBOL_BINDING_MASK;
   }
 
   bool isHidden() const {

llvm/trunk/lib/MC/WasmObjectWriter.cpp

 struct WasmExport {
   StringRef FieldName;
   unsigned Kind;
   uint32_t Index;
 };
 
 // A wasm global to be written into the global section.
 struct WasmGlobal {
-  wasm::ValType Type;
-  bool IsMutable;
-  bool HasImport;
+  wasm::WasmGlobalType Type;
   uint64_t InitialValue;
-  uint32_t ImportIndex;
 };
 
 // Information about a single item which is part of a COMDAT.  For each data
 // segment or function which is in the COMDAT, there is a corresponding
 // WasmComdatEntry.
 struct WasmComdatEntry {
   unsigned Kind;
   uint32_t Index;
void WasmObjectWriter::writeGlobalSection() {
   if (Globals.empty())
     return;
 
   SectionBookkeeping Section;
   startSection(Section, wasm::WASM_SEC_GLOBAL);
 
   encodeULEB128(Globals.size(), getStream());
   for (const WasmGlobal &Global : Globals) {
-    writeValueType(Global.Type);
-    write8(Global.IsMutable);
+    writeValueType(static_cast<wasm::ValType>(Global.Type.Type));
+    write8(Global.Type.Mutable);
 
-    if (Global.HasImport) {
-      assert(Global.InitialValue == 0);
-      write8(wasm::WASM_OPCODE_GET_GLOBAL);
-      encodeULEB128(Global.ImportIndex, getStream());
-    } else {
-      assert(Global.ImportIndex == 0);
-      write8(wasm::WASM_OPCODE_I32_CONST);
-      encodeSLEB128(Global.InitialValue, getStream()); // offset
-    }
+    encodeSLEB128(Global.InitialValue, getStream());
     write8(wasm::WASM_OPCODE_END);
   }
 
   endSection(Section);
 }
 
 void WasmObjectWriter::writeExportSection(ArrayRef<WasmExport> Exports) {
   if (Exports.empty())
uint32_t WasmObjectWriter::registerFunctionType(const MCSymbolWasm& Symbol) {
   DEBUG(dbgs() << "  -> type index: " << Pair.first->second << "\n");
   return Pair.first->second;
 }
 
 void WasmObjectWriter::writeObject(MCAssembler &Asm,
                                    const MCAsmLayout &Layout) {
   DEBUG(dbgs() << "WasmObjectWriter::writeObject\n");
   MCContext &Ctx = Asm.getContext();
-  wasm::ValType PtrType = is64Bit() ? wasm::ValType::I64 : wasm::ValType::I32;
+  int32_t PtrType = is64Bit() ? wasm::WASM_TYPE_I64 : wasm::WASM_TYPE_I32;
 
   // Collect information from the available symbols.
   SmallVector<WasmFunction, 4> Functions;
   SmallVector<uint32_t, 4> TableElems;
   SmallVector<WasmImport, 4> Imports;
   SmallVector<WasmExport, 4> Exports;
   SmallVector<std::pair<StringRef, uint32_t>, 4> SymbolFlags;
   SmallVector<std::pair<uint16_t, uint32_t>, 2> InitFuncs;
   std::map<StringRef, std::vector<WasmComdatEntry>> Comdats;
   SmallVector<WasmDataSegment, 4> DataSegments;
   uint32_t DataSize = 0;
 
-  // In the special .global_variables section, we've encoded global
-  // variables used by the function. Translate them into the Globals
-  // list.
-  MCSectionWasm *GlobalVars =
-      Ctx.getWasmSection(".global_variables", SectionKind::getMetadata());
-  if (!GlobalVars->getFragmentList().empty()) {
-    if (GlobalVars->getFragmentList().size() != 1)
-      report_fatal_error("only one .global_variables fragment supported");
-    const MCFragment &Frag = *GlobalVars->begin();
-    if (Frag.hasInstructions() || Frag.getKind() != MCFragment::FT_Data)
-      report_fatal_error("only data supported in .global_variables");
-    const auto &DataFrag = cast<MCDataFragment>(Frag);
-    if (!DataFrag.getFixups().empty())
-      report_fatal_error("fixups not supported in .global_variables");
-    const SmallVectorImpl<char> &Contents = DataFrag.getContents();
-    for (const uint8_t *p = (const uint8_t *)Contents.data(),
-                     *end = (const uint8_t *)Contents.data() + Contents.size();
-         p != end; ) {
-      WasmGlobal G;
-      if (end - p < 3)
-        report_fatal_error("truncated global variable encoding");
-      G.Type = wasm::ValType(int8_t(*p++));
-      G.IsMutable = bool(*p++);
-      G.HasImport = bool(*p++);
-      if (G.HasImport) {
-        G.InitialValue = 0;
-
-        WasmImport Import;
-        Import.ModuleName = (const char *)p;
-        const uint8_t *nul = (const uint8_t *)memchr(p, '\0', end - p);
-        if (!nul)
-          report_fatal_error("global module name must be nul-terminated");
-        p = nul + 1;
-        nul = (const uint8_t *)memchr(p, '\0', end - p);
-        if (!nul)
-          report_fatal_error("global base name must be nul-terminated");
-        Import.FieldName = (const char *)p;
-        p = nul + 1;
-
-        Import.Kind = wasm::WASM_EXTERNAL_GLOBAL;
-        Import.Type = int32_t(G.Type);
-
-        G.ImportIndex = NumGlobalImports;
-        ++NumGlobalImports;
-
-        Imports.push_back(Import);
-      } else {
-        unsigned n;
-        G.InitialValue = decodeSLEB128(p, &n);
-        G.ImportIndex = 0;
-        if ((ptrdiff_t)n > end - p)
-          report_fatal_error("global initial value must be valid SLEB128");
-        p += n;
-      }
-      Globals.push_back(G);
-    }
-  }
-
   // For now, always emit the memory import, since loads and stores are not
   // valid without it. In the future, we could perhaps be more clever and omit
   // it if there are no loads or stores.
   MCSymbolWasm *MemorySym =
       cast<MCSymbolWasm>(Ctx.getOrCreateSymbol("__linear_memory"));
   WasmImport MemImport;
   MemImport.ModuleName = MemorySym->getModuleName();
   MemImport.FieldName = MemorySym->getName();
if ((!WS.isDefined() && !WS.isComdat()) ||
 
       if (WS.isFunction()) {
         Import.Kind = wasm::WASM_EXTERNAL_FUNCTION;
         Import.Type = getFunctionType(WS);
         SymbolIndices[&WS] = NumFunctionImports;
         ++NumFunctionImports;
       } else {
         Import.Kind = wasm::WASM_EXTERNAL_GLOBAL;
-        Import.Type = int32_t(PtrType);
+        Import.Type = PtrType;
         Import.IsMutable = false;
         SymbolIndices[&WS] = NumGlobalImports;
 
         // If this global is the stack pointer, make it mutable.
         if (WS.getName() == "__stack_pointer")
           Import.IsMutable = true;
 
         ++NumGlobalImports;
if (WS.isFunction()) {
 
       // For each global, prepare a corresponding wasm global holding its
       // address.  For externals these will also be named exports.
       Index = NumGlobalImports + Globals.size();
       auto &DataSection = static_cast<MCSectionWasm &>(WS.getSection());
       assert(DataSection.isWasmData());
 
       WasmGlobal Global;
-      Global.Type = PtrType;
-      Global.IsMutable = false;
-      Global.HasImport = false;
+      Global.Type.Type = PtrType;
+      Global.Type.Mutable = false;
       Global.InitialValue = DataSection.getMemoryOffset() + Layout.getSymbolOffset(WS);
-      Global.ImportIndex = 0;
       SymbolIndices[&WS] = Index;
       DEBUG(dbgs() << "  -> global index: " << Index << "\n");
       Globals.push_back(Global);
     }
 
     // If the symbol is visible outside this translation unit, export it.
     if (WS.isDefined()) {
       WasmExport Export;

llvm/trunk/lib/Object/WasmObjectFile.cpp

 }
 
 Error WasmObjectFile::parseGlobalSection(const uint8_t *Ptr, const uint8_t *End) {
   uint32_t Count = readVaruint32(Ptr);
   Globals.reserve(Count);
   while (Count--) {
     wasm::WasmGlobal Global;
     Global.Index = NumImportedGlobals + Globals.size();
-    Global.Type = readVarint7(Ptr);
-    Global.Mutable = readVaruint1(Ptr);
+    Global.Type.Type = readVarint7(Ptr);
+    Global.Type.Mutable = readVaruint1(Ptr);
     if (Error Err = readInitExpr(Global.InitExpr, Ptr))
       return Err;
     Globals.push_back(Global);
   }
   if (Ptr != End)
     return make_error<GenericBinaryError>("Global section ended prematurely",
                                           object_error::parse_failed);
   return Error::success();

 
 void WasmObjectFile::moveSymbolNext(DataRefImpl &Symb) const { Symb.d.a++; }
 
 uint32_t WasmObjectFile::getSymbolFlags(DataRefImpl Symb) const {
   uint32_t Result = SymbolRef::SF_None;
   const WasmSymbol &Sym = getWasmSymbol(Symb);
 
   DEBUG(dbgs() << "getSymbolFlags: ptr=" << &Sym << " " << Sym << "\n");
-  if (Sym.isWeak())
+  if (Sym.isBindingWeak())
     Result |= SymbolRef::SF_Weak;
-  if (!Sym.isLocal())
+  if (!Sym.isBindingLocal())
     Result |= SymbolRef::SF_Global;
   if (Sym.isHidden())
     Result |= SymbolRef::SF_Hidden;
 
   switch (Sym.Type) {
   case WasmSymbol::SymbolType::FUNCTION_IMPORT:
     Result |= SymbolRef::SF_Undefined | SymbolRef::SF_Executable;
     break;

llvm/trunk/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.h

public:
   explicit WebAssemblyTargetStreamer(MCStreamer &S);
 
   /// .param
   virtual void emitParam(MCSymbol *Symbol, ArrayRef<MVT> Types) = 0;
   /// .result
   virtual void emitResult(MCSymbol *Symbol, ArrayRef<MVT> Types) = 0;
   /// .local
   virtual void emitLocal(ArrayRef<MVT> Types) = 0;
-  /// .globalvar
-  virtual void emitGlobal(ArrayRef<wasm::Global> Globals) = 0;
   /// .endfunc
   virtual void emitEndFunc() = 0;
   /// .functype
   virtual void emitIndirectFunctionType(MCSymbol *Symbol,
                                         SmallVectorImpl<MVT> &Params,
                                         SmallVectorImpl<MVT> &Results) = 0;
   /// .indidx
   virtual void emitIndIdx(const MCExpr *Value) = 0;
class WebAssemblyTargetAsmStreamer final : public WebAssemblyTargetStreamer {
   formatted_raw_ostream &OS;
 
 public:
   WebAssemblyTargetAsmStreamer(MCStreamer &S, formatted_raw_ostream &OS);
 
   void emitParam(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitResult(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitLocal(ArrayRef<MVT> Types) override;
-  void emitGlobal(ArrayRef<wasm::Global> Globals) override;
   void emitEndFunc() override;
   void emitIndirectFunctionType(MCSymbol *Symbol,
                                 SmallVectorImpl<MVT> &Params,
                                 SmallVectorImpl<MVT> &Results) override;
   void emitIndIdx(const MCExpr *Value) override;
   void emitGlobalImport(StringRef name) override;
 };
 
 /// This part is for ELF object output
 class WebAssemblyTargetELFStreamer final : public WebAssemblyTargetStreamer {
 public:
   explicit WebAssemblyTargetELFStreamer(MCStreamer &S);
 
   void emitParam(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitResult(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitLocal(ArrayRef<MVT> Types) override;
-  void emitGlobal(ArrayRef<wasm::Global> Globals) override;
   void emitEndFunc() override;
   void emitIndirectFunctionType(MCSymbol *Symbol,
                                 SmallVectorImpl<MVT> &Params,
                                 SmallVectorImpl<MVT> &Results) override;
   void emitIndIdx(const MCExpr *Value) override;
   void emitGlobalImport(StringRef name) override;
 };
 
 /// This part is for Wasm object output
 class WebAssemblyTargetWasmStreamer final : public WebAssemblyTargetStreamer {
 public:
   explicit WebAssemblyTargetWasmStreamer(MCStreamer &S);
 
   void emitParam(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitResult(MCSymbol *Symbol, ArrayRef<MVT> Types) override;
   void emitLocal(ArrayRef<MVT> Types) override;
-  void emitGlobal(ArrayRef<wasm::Global> Globals) override;
   void emitEndFunc() override;
   void emitIndirectFunctionType(MCSymbol *Symbol,
                                 SmallVectorImpl<MVT> &Params,
                                 SmallVectorImpl<MVT> &Results) override;
   void emitIndIdx(const MCExpr *Value) override;
   void emitGlobalImport(StringRef name) override;
 };
 
 } // end namespace llvm
 
 #endif

llvm/trunk/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyTargetStreamer.cpp

 
 void WebAssemblyTargetAsmStreamer::emitLocal(ArrayRef<MVT> Types) {
   if (!Types.empty()) {
     OS << "\t.local  \t";
     PrintTypes(OS, Types);
   }
 }
 
-void WebAssemblyTargetAsmStreamer::emitGlobal(
-    ArrayRef<wasm::Global> Globals) {
-  if (!Globals.empty()) {
-    OS << "\t.globalvar  \t";
-
-    bool First = true;
-    for (const wasm::Global &G : Globals) {
-      if (First)
-        First = false;
-      else
-        OS << ", ";
-      OS << WebAssembly::TypeToString(G.Type);
-      if (!G.InitialModule.empty())
-        OS << '=' << G.InitialModule << ':' << G.InitialName;
-      else
-        OS << '=' << G.InitialValue;
-    }
-    OS << '\n';
-  }
-}
-
 void WebAssemblyTargetAsmStreamer::emitEndFunc() { OS << "\t.endfunc\n"; }
 
 void WebAssemblyTargetAsmStreamer::emitIndirectFunctionType(
     MCSymbol *Symbol, SmallVectorImpl<MVT> &Params, SmallVectorImpl<MVT> &Results) {
   OS << "\t.functype\t" << Symbol->getName();
   if (Results.empty())
     OS << ", void";
   else {

 }
 
 void WebAssemblyTargetELFStreamer::emitLocal(ArrayRef<MVT> Types) {
   Streamer.EmitULEB128IntValue(Types.size());
   for (MVT Type : Types)
     emitValueType(WebAssembly::toValType(Type));
 }
 
-void WebAssemblyTargetELFStreamer::emitGlobal(
-    ArrayRef<wasm::Global> Globals) {
-  llvm_unreachable(".globalvar encoding not yet implemented");
-}
-
 void WebAssemblyTargetELFStreamer::emitEndFunc() {
   Streamer.EmitIntValue(WebAssembly::End, 1);
 }
 
 void WebAssemblyTargetELFStreamer::emitIndIdx(const MCExpr *Value) {
   llvm_unreachable(".indidx encoding not yet implemented");
 }
 
void WebAssemblyTargetWasmStreamer::emitLocal(ArrayRef<MVT> Types) {
 
   Streamer.EmitULEB128IntValue(Grouped.size());
   for (auto Pair : Grouped) {
     Streamer.EmitULEB128IntValue(Pair.second);
     emitValueType(WebAssembly::toValType(Pair.first));
   }
 }
 
-void WebAssemblyTargetWasmStreamer::emitGlobal(
-    ArrayRef<wasm::Global> Globals) {
-  // Encode the globals use by the funciton into the special .global_variables
-  // section. This will later be decoded and turned into contents for the
-  // Globals Section.
-  Streamer.PushSection();
-  Streamer.SwitchSection(Streamer.getContext().getWasmSection(
-      ".global_variables", SectionKind::getMetadata()));
-  for (const wasm::Global &G : Globals) {
-    Streamer.EmitIntValue(int32_t(G.Type), 1);
-    Streamer.EmitIntValue(G.Mutable, 1);
-    if (G.InitialModule.empty()) {
-      Streamer.EmitIntValue(0, 1); // indicate that we have an int value
-      Streamer.EmitSLEB128IntValue(0);
-    } else {
-      Streamer.EmitIntValue(1, 1); // indicate that we have a module import
-      Streamer.EmitBytes(G.InitialModule);
-      Streamer.EmitIntValue(0, 1); // nul-terminate
-      Streamer.EmitBytes(G.InitialName);
-      Streamer.EmitIntValue(0, 1); // nul-terminate
-    }
-  }
-  Streamer.PopSection();
-}
-
 void WebAssemblyTargetWasmStreamer::emitEndFunc() {
   llvm_unreachable(".end_func is not needed for direct wasm output");
 }
 
 void WebAssemblyTargetWasmStreamer::emitIndIdx(const MCExpr *Value) {
   llvm_unreachable(".indidx encoding not yet implemented");
 }
 

llvm/trunk/tools/obj2yaml/wasm2yaml.cpp

case wasm::WASM_SEC_MEMORY: {
       S = std::move(MemorySec);
       break;
     }
     case wasm::WASM_SEC_GLOBAL: {
       auto GlobalSec = make_unique<WasmYAML::GlobalSection>();
       for (auto &Global : Obj.globals()) {
         WasmYAML::Global G;
         G.Index = Global.Index;
-        G.Type = Global.Type;
-        G.Mutable = Global.Mutable;
+        G.Type = Global.Type.Type;
+        G.Mutable = Global.Type.Mutable;
         G.InitExpr = Global.InitExpr;
         GlobalSec->Globals.push_back(G);
       }
       S = std::move(GlobalSec);
       break;
     }
     case wasm::WASM_SEC_START: {
       auto StartSec = make_unique<WasmYAML::StartSection>();