[WebAssembly] Make WasmSymbol's signature usable for events (NFC)

[aheejin]

Summary

WasmSignature used to use its WasmSignature member variable only for
function types, but now it also can be used for events as well.

Companion to D54875.

[sbc100]

Should the first word of the description be WasmSymbol

include/llvm/BinaryFormat/Wasm.h

335 ↗

(On Diff #176541)

Did you mean to remove this?

EventTypes are not equal unless the signature is equal too right?

[aheejin]

include/llvm/BinaryFormat/Wasm.h

335 ↗

(On Diff #176541)

This was supposed to be used in checkEventType() function in lld patch (D54875). When parsing multiple object files and adding events from the object files, the function is used to check the pre-existing event type with the same name has the same attribute and signature with the new event type. At that point, it is adding events from several objects and the same SigIndex does not mean anything, because you can't compare sigindex of different objects. This was one of the reasons that I wanted to take SigIndex out of WasmEventType in D54873.

Anyway, I just checked the current state of D54873 and it is not currently using this operator but comparing Attribute field directly, like this:

const WasmEventType *OldType = cast<EventSymbol>(Existing)->getEventType();
const WasmSignature *OldSig = ExistingEvent->Signature;
if (NewType->Attribute != OldType->Attribute)
  error("Event type mismatch: " + Existing->getName() + "\n>>> defined as " +
        toString(*OldType) + " in " + toString(Existing->getFile()) +
        "\n>>> defined as " + toString(*NewType) + " in " + toString(File));
if (*NewSig != *OldSig)
  warn("Event signature mismatch: " + Existing->getName() +
       "\n>>> defined as " + toString(*OldSig) + " in " +
       toString(Existing->getFile()) + "\n>>> defined as " +
       toString(*NewSig) + " in " + toString(File));

So I think I can actually delete this operator.

[sbc100]

include/llvm/BinaryFormat/Wasm.h

335 ↗

(On Diff #176541)

Thats sounds like the right direction yes.

[aheejin]

• Delete unused operators

[sbc100]

IIUC this change is NFC so you can mark it as such in the title?

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

 inline bool operator==(const WasmGlobalType &LHS, const WasmGlobalType &RHS) {
   return LHS.Type == RHS.Type && LHS.Mutable == RHS.Mutable;
 }
 
 inline bool operator!=(const WasmGlobalType &LHS, const WasmGlobalType &RHS) {
   return !(LHS == RHS);
 }
 
-inline bool operator==(const WasmEventType &LHS, const WasmEventType &RHS) {
-  return LHS.Attribute == RHS.Attribute && LHS.SigIndex == RHS.SigIndex;
-}
-
-inline bool operator!=(const WasmEventType &LHS, const WasmEventType &RHS) {
-  return !(LHS == RHS);
-}
-
 std::string toString(wasm::WasmSymbolType type);
 std::string relocTypetoString(uint32_t type);
 
 } // end namespace wasm
 } // end namespace llvm
 
 #endif

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

 #include <vector>
 
 namespace llvm {
 namespace object {
 
 class WasmSymbol {
 public:
   WasmSymbol(const wasm::WasmSymbolInfo &Info,
-             const wasm::WasmSignature *FunctionType,
              const wasm::WasmGlobalType *GlobalType,
-             const wasm::WasmEventType *EventType)
-      : Info(Info), FunctionType(FunctionType), GlobalType(GlobalType),
-        EventType(EventType) {}
+             const wasm::WasmEventType *EventType,
+             const wasm::WasmSignature *Signature)
+      : Info(Info), GlobalType(GlobalType), EventType(EventType),
+        Signature(Signature) {}
 
   const wasm::WasmSymbolInfo &Info;
-  const wasm::WasmSignature *FunctionType;
   const wasm::WasmGlobalType *GlobalType;
   const wasm::WasmEventType *EventType;
+  const wasm::WasmSignature *Signature;
 
   bool isTypeFunction() const {
     return Info.Kind == wasm::WASM_SYMBOL_TYPE_FUNCTION;
   }
 
   bool isTypeData() const { return Info.Kind == wasm::WASM_SYMBOL_TYPE_DATA; }
 
   bool isTypeGlobal() const {

llvm/trunk/lib/Object/WasmObjectFile.cpp

for (auto &I : Imports) {
     else if (I.Kind == wasm::WASM_EXTERNAL_GLOBAL)
       ImportedGlobals.emplace_back(&I);
     else if (I.Kind == wasm::WASM_EXTERNAL_EVENT)
       ImportedEvents.emplace_back(&I);
   }
 
   while (Count--) {
     wasm::WasmSymbolInfo Info;
-    const wasm::WasmSignature *FunctionType = nullptr;
+    const wasm::WasmSignature *Signature = nullptr;
     const wasm::WasmGlobalType *GlobalType = nullptr;
     const wasm::WasmEventType *EventType = nullptr;
 
     Info.Kind = readUint8(Ctx);
     Info.Flags = readVaruint32(Ctx);
     bool IsDefined = (Info.Flags & wasm::WASM_SYMBOL_UNDEFINED) == 0;
 
     switch (Info.Kind) {
     case wasm::WASM_SYMBOL_TYPE_FUNCTION:
       Info.ElementIndex = readVaruint32(Ctx);
       if (!isValidFunctionIndex(Info.ElementIndex) ||
           IsDefined != isDefinedFunctionIndex(Info.ElementIndex))
         return make_error<GenericBinaryError>("invalid function symbol index",
                                               object_error::parse_failed);
       if (IsDefined) {
         Info.Name = readString(Ctx);
         unsigned FuncIndex = Info.ElementIndex - NumImportedFunctions;
-        FunctionType = &Signatures[FunctionTypes[FuncIndex]];
+        Signature = &Signatures[FunctionTypes[FuncIndex]];
         wasm::WasmFunction &Function = Functions[FuncIndex];
         if (Function.SymbolName.empty())
           Function.SymbolName = Info.Name;
       } else {
         wasm::WasmImport &Import = *ImportedFunctions[Info.ElementIndex];
-        FunctionType = &Signatures[Import.SigIndex];
+        Signature = &Signatures[Import.SigIndex];
         Info.Name = Import.Field;
         Info.Module = Import.Module;
       }
       break;
 
     case wasm::WASM_SYMBOL_TYPE_GLOBAL:
       Info.ElementIndex = readVaruint32(Ctx);
       if (!isValidGlobalIndex(Info.ElementIndex) ||
case wasm::WASM_SYMBOL_TYPE_EVENT: {
       if (!IsDefined && (Info.Flags & wasm::WASM_SYMBOL_BINDING_MASK) ==
                             wasm::WASM_SYMBOL_BINDING_WEAK)
         return make_error<GenericBinaryError>("undefined weak global symbol",
                                               object_error::parse_failed);
       if (IsDefined) {
         Info.Name = readString(Ctx);
         unsigned EventIndex = Info.ElementIndex - NumImportedEvents;
         wasm::WasmEvent &Event = Events[EventIndex];
+        Signature = &Signatures[Event.Type.SigIndex];
         EventType = &Event.Type;
         if (Event.SymbolName.empty())
           Event.SymbolName = Info.Name;
 
       } else {
         wasm::WasmImport &Import = *ImportedEvents[Info.ElementIndex];
         EventType = &Import.Event;
+        Signature = &Signatures[EventType->SigIndex];
         Info.Name = Import.Field;
       }
       break;
     }
 
     default:
       return make_error<GenericBinaryError>("Invalid symbol type",
                                             object_error::parse_failed);
     }
 
     if ((Info.Flags & wasm::WASM_SYMBOL_BINDING_MASK) !=
             wasm::WASM_SYMBOL_BINDING_LOCAL &&
         !SymbolNames.insert(Info.Name).second)
       return make_error<GenericBinaryError>("Duplicate symbol name " +
                                                 Twine(Info.Name),
                                             object_error::parse_failed);
     LinkingData.SymbolTable.emplace_back(Info);
-    Symbols.emplace_back(LinkingData.SymbolTable.back(), FunctionType,
-                         GlobalType, EventType);
+    Symbols.emplace_back(LinkingData.SymbolTable.back(), GlobalType, EventType,
+                         Signature);
     LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n");
   }
 
   return Error::success();
 }
 
 Error WasmObjectFile::parseLinkingSectionComdat(ReadContext &Ctx) {
   uint32_t ComdatCount = readVaruint32(Ctx);