[lld][WebAssembly] Cleanup duplicate fields in Symbols.h. NFC

[sbc100]

Summary

This avoids duplication and simplifies the code in several places
without increasing the size of the symbol union (at least not
above the assert'd limit of 120 bytes).

[sbc100]

revert part

[fitzgen]

lld/wasm/Symbols.h

177

Are you concerned about growing the size of Symbol by 6 words, even though the majority of symbols aren't imported and won't use these fields?

[fitzgen]

lld/wasm/Symbols.h

177

Ah okay I see what you mean about this not growing the size of SymbolUnion which I guess is more important than Symbol itself.

[sbc100]

lld/wasm/Symbols.h

177

Its only the sizeof SymbolUnion that actually makes any difference here since all symbols are allocated at that size. This change didn't effect the sizeof SymbolUnion, at least no enough to cause the static assert to fail. I can take a look at weather it changed at all.

[sbc100]

lld/wasm/Symbols.h

177

Confirms that SymbolUnion size if unchanged by this PR (120 bytes before and after).

[sbc100]

• rebase

lld/wasm/Symbols.h

public:
   // called) and should otherwise be treated as missing/undefined.  See
   // SymbolTable::replaceWithUndefined.
   // These stubs never appear in the table and any table index relocations
   // against them will produce address 0 (The table index representing
   // the null function pointer).
   bool isStub : 1;
 
   uint32_t flags;
+
+  llvm::Optional<StringRef> importName;
+  llvm::Optional<StringRef> importModule;
 };
 
 class FunctionSymbol : public Symbol {
 public:
   static bool classof(const Symbol *s) {
     return s->kind() == DefinedFunctionKind ||
            s->kind() == UndefinedFunctionKind;
   }

 class UndefinedFunction : public FunctionSymbol {
 public:
   UndefinedFunction(StringRef name, llvm::Optional<StringRef> importName,
                     llvm::Optional<StringRef> importModule, uint32_t flags,
                     InputFile *file = nullptr,
                     const WasmSignature *type = nullptr,
                     bool isCalledDirectly = true)
       : FunctionSymbol(name, UndefinedFunctionKind, flags, file, type),
-        importName(importName), importModule(importModule),
-        isCalledDirectly(isCalledDirectly) {}
+        isCalledDirectly(isCalledDirectly) {
+    this->importName = importName;
+    this->importModule = importModule;
+  }
 
   static bool classof(const Symbol *s) {
     return s->kind() == UndefinedFunctionKind;
   }
 
-  llvm::Optional<StringRef> importName;
-  llvm::Optional<StringRef> importModule;
   DefinedFunction *stubFunction = nullptr;
   bool isCalledDirectly;
 };
 
 // Section symbols for output sections are different from those for input
 // section.  These are generated by the linker and point the OutputSection
 // rather than an InputSection.
 class OutputSectionSymbol : public Symbol {

 };
 
 class UndefinedGlobal : public GlobalSymbol {
 public:
   UndefinedGlobal(StringRef name, llvm::Optional<StringRef> importName,
                   llvm::Optional<StringRef> importModule, uint32_t flags,
                   InputFile *file = nullptr,
                   const WasmGlobalType *type = nullptr)
-      : GlobalSymbol(name, UndefinedGlobalKind, flags, file, type),
-        importName(importName), importModule(importModule) {}
+      : GlobalSymbol(name, UndefinedGlobalKind, flags, file, type) {
+    this->importName = importName;
+    this->importModule = importModule;
+  }
 
   static bool classof(const Symbol *s) {
     return s->kind() == UndefinedGlobalKind;
   }
-
-  llvm::Optional<StringRef> importName;
-  llvm::Optional<StringRef> importModule;
 };
 
 class TableSymbol : public Symbol {
 public:
   static bool classof(const Symbol *s) {
     return s->kind() == DefinedTableKind || s->kind() == UndefinedTableKind;
   }
 
public:
   InputTable *table;
 };
 
 class UndefinedTable : public TableSymbol {
 public:
   UndefinedTable(StringRef name, llvm::Optional<StringRef> importName,
                  llvm::Optional<StringRef> importModule, uint32_t flags,
                  InputFile *file, const WasmTableType *type)
-      : TableSymbol(name, UndefinedTableKind, flags, file, type),
-        importName(importName), importModule(importModule) {}
+      : TableSymbol(name, UndefinedTableKind, flags, file, type) {
+    this->importName = importName;
+    this->importModule = importModule;
+  }
 
   static bool classof(const Symbol *s) {
     return s->kind() == UndefinedTableKind;
   }
-
-  llvm::Optional<StringRef> importName;
-  llvm::Optional<StringRef> importModule;
 };
 
 // A tag is a general format to distinguish typed entities. Each tag has an
 // attribute and a type. Currently the attribute can only specify that the tag
 // is for an exception tag.
 //
 // In exception handling, tags are used to distinguish different kinds of
 // exceptions. For example, they can be used to distinguish different language's

lld/wasm/SyntheticSections.cpp

void ImportSection::addGOTEntry(Symbol *sym) {
   assert(!isSealed);
   if (sym->hasGOTIndex())
     return;
   LLVM_DEBUG(dbgs() << "addGOTEntry: " << toString(*sym) << "\n");
   sym->setGOTIndex(numImportedGlobals++);
   gotSymbols.push_back(sym);
 }
 
-template <typename UndefinedT>
-static void getImportModuleAndName(Symbol *sym, StringRef *outModule,
-                                   StringRef *outName) {
-  if (auto *undef = dyn_cast<UndefinedT>(sym)) {
-    *outModule = undef->importModule ? *undef->importModule : defaultModule;
-    *outName = undef->importName ? *undef->importName : sym->getName();
-  } else {
-    *outModule = defaultModule;
-    *outName = sym->getName();
-  }
-}
-
 void ImportSection::addImport(Symbol *sym) {
   assert(!isSealed);
-  StringRef module;
-  StringRef name;
+  StringRef module = sym->importModule.getValueOr(defaultModule);
+  StringRef name = sym->importName.getValueOr(sym->getName());
   if (auto *f = dyn_cast<FunctionSymbol>(sym)) {
-    getImportModuleAndName<UndefinedFunction>(sym, &module, &name);
     ImportKey<WasmSignature> key(*(f->getSignature()), module, name);
     auto entry = importedFunctions.try_emplace(key, numImportedFunctions);
     if (entry.second) {
       importedSymbols.emplace_back(sym);
       f->setFunctionIndex(numImportedFunctions++);
     } else {
       f->setFunctionIndex(entry.first->second);
     }
   } else if (auto *g = dyn_cast<GlobalSymbol>(sym)) {
-    getImportModuleAndName<UndefinedGlobal>(sym, &module, &name);
     ImportKey<WasmGlobalType> key(*(g->getGlobalType()), module, name);
     auto entry = importedGlobals.try_emplace(key, numImportedGlobals);
     if (entry.second) {
       importedSymbols.emplace_back(sym);
       g->setGlobalIndex(numImportedGlobals++);
     } else {
       g->setGlobalIndex(entry.first->second);
     }
   } else if (auto *t = dyn_cast<TagSymbol>(sym)) {
     // NB: There's currently only one possible kind of tag, and no
     // `UndefinedTag`, so we don't bother de-duplicating tag imports.
     importedSymbols.emplace_back(sym);
     t->setTagIndex(numImportedTags++);
   } else {
     auto *table = cast<TableSymbol>(sym);
-    getImportModuleAndName<UndefinedTable>(sym, &module, &name);
     ImportKey<WasmTableType> key(*(table->getTableType()), module, name);
     auto entry = importedTables.try_emplace(key, numImportedTables);
     if (entry.second) {
       importedSymbols.emplace_back(sym);
       table->setTableNumber(numImportedTables++);
     } else {
       table->setTableNumber(entry.first->second);
     }
if (config->sharedMemory)
       import.Memory.Flags |= WASM_LIMITS_FLAG_IS_SHARED;
     if (is64)
       import.Memory.Flags |= WASM_LIMITS_FLAG_IS_64;
     writeImport(os, import);
   }
 
   for (const Symbol *sym : importedSymbols) {
     WasmImport import;
-    if (auto *f = dyn_cast<UndefinedFunction>(sym)) {
-      import.Field = f->importName ? *f->importName : sym->getName();
-      import.Module = f->importModule ? *f->importModule : defaultModule;
-    } else if (auto *g = dyn_cast<UndefinedGlobal>(sym)) {
-      import.Field = g->importName ? *g->importName : sym->getName();
-      import.Module = g->importModule ? *g->importModule : defaultModule;
-    } else if (auto *t = dyn_cast<UndefinedTable>(sym)) {
-      import.Field = t->importName ? *t->importName : sym->getName();
-      import.Module = t->importModule ? *t->importModule : defaultModule;
-    } else {
-      import.Field = sym->getName();
-      import.Module = defaultModule;
-    }
+    import.Field = sym->importName.getValueOr(sym->getName());
+    import.Module = sym->importModule.getValueOr(defaultModule);
 
     if (auto *functionSym = dyn_cast<FunctionSymbol>(sym)) {
       import.Kind = WASM_EXTERNAL_FUNCTION;
       import.SigIndex = out.typeSec->lookupType(*functionSym->signature);
     } else if (auto *globalSym = dyn_cast<GlobalSymbol>(sym)) {
       import.Kind = WASM_EXTERNAL_GLOBAL;
       import.Global = *globalSym->getGlobalType();
     } else if (auto *tagSym = dyn_cast<TagSymbol>(sym)) {

lld/wasm/Writer.cpp

static bool shouldImport(Symbol *sym) {
   // as GOT entries.
   if (isa<DataSymbol>(sym))
     return false;
 
   if (config->isPic || config->relocatable || config->importUndefined)
     return true;
   if (config->allowUndefinedSymbols.count(sym->getName()) != 0)
     return true;
-  if (auto *g = dyn_cast<UndefinedGlobal>(sym))
-    return g->importName.hasValue();
-  if (auto *f = dyn_cast<UndefinedFunction>(sym))
-    return f->importName.hasValue();
-  if (auto *t = dyn_cast<UndefinedTable>(sym))
-    return t->importName.hasValue();
+  if (sym->isUndefined())
+    return sym->importName.hasValue();
 
   return false;
 }
 
 void Writer::calculateImports() {
   // Some inputs require that the indirect function table be assigned to table
   // number 0, so if it is present and is an import, allocate it before any
   // other tables.