[WebAssembly] Simplify use of InputChunk within Symbols. NFC.

[sbc100]

Summary

Simplify the code so that we can use in InputChunk pointer
rather than an InputFunction + InputSegment depending
on the symbol type.

Use llvm dynamic casts when we need a particular subtype.

This change is useful for the upcoming gc-section support
which wants to deal with all input chunks in the same way.

[sbc100]

• clang format

[sbc100]

Sorry, I didn't mean to commit this without lgtm. I made a git-svn mistake. Please review and I will revert or update as needed.

[ncw]

LGTM! I'm happy - I even added InputGlobal as a third type, which makes it even uglier, so this looks good. I even added a Symbol::getChunk method that's similar to yours (but not as tidy).

Rebasing my patches shouldn't be a problem. One thing I did play around with was trying to store a pointer to the Symbol inside the InputChunk - but I quickly abandoned that, it won't work (since some chunks don't have an associated symbol, and some chunks have several associated symbols, and symbols can be unassociated from chunks when weak symbols are overridden...). Just an FYI in case you're ever tempted to try the same thing! Symbol ➝ chunk is good, chunk ➝ symbol is bad.

[ruiu]

LGTM

wasm/SymbolTable.cpp

124

nit: WasmSignature* Sig -> WasmSignature *Sig

wasm/Symbols.cpp

46

auto* -> auto *.

wasm/InputChunks.h

 namespace lld {
 namespace wasm {
 
 class ObjFile;
 class OutputSegment;
 
 class InputChunk {
 public:
+  enum Kind { DataSegment, Function };
+
+  Kind kind() const { return SectionKind; };
+
   uint32_t getSize() const { return data().size(); }
 
   void copyRelocations(const WasmSection &Section);
 
   void writeTo(uint8_t *SectionStart) const;
 
   void setOutputOffset(uint32_t Offset) {
     OutputOffset = Offset;
     calcRelocations();
   }
 
   uint32_t getOutputOffset() const { return OutputOffset; }
   ArrayRef<WasmRelocation> getRelocations() const { return Relocations; }
 
   virtual StringRef getComdat() const = 0;
 
   bool Discarded = false;
   std::vector<OutputRelocation> OutRelocations;
 
 protected:
-  InputChunk(const ObjFile *F) : File(F) {}
+  InputChunk(const ObjFile *F, Kind K) : File(F), SectionKind(K) {}
   virtual ~InputChunk() = default;
   void calcRelocations();
   virtual ArrayRef<uint8_t> data() const = 0;
   virtual uint32_t getInputSectionOffset() const = 0;
 
   std::vector<WasmRelocation> Relocations;
   int32_t OutputOffset = 0;
   const ObjFile *File;
+  Kind SectionKind;
 };
 
 // Represents a WebAssembly data segment which can be included as part of
 // an output data segments.  Note that in WebAssembly, unlike ELF and other
 // formats, used the term "data segment" to refer to the continous regions of
 // memory that make on the data section. See:
 // https://webassembly.github.io/spec/syntax/modules.html#syntax-data
 //
 // For example, by default, clang will produce a separate data section for
 // each global variable.
 class InputSegment : public InputChunk {
 public:
   InputSegment(const WasmSegment &Seg, const ObjFile *F)
-      : InputChunk(F), Segment(Seg) {}
+      : InputChunk(F, InputChunk::DataSegment), Segment(Seg) {}
+
+  static bool classof(const InputChunk *C) { return C->kind() == DataSegment; }
 
   // Translate an offset in the input segment to an offset in the output
   // segment.
   uint32_t translateVA(uint32_t Address) const;
 
   const OutputSegment *getOutputSegment() const { return OutputSeg; }
 
   void setOutputSegment(const OutputSegment *Segment, uint32_t Offset) {

 };
 
 // Represents a single wasm function within and input file.  These are
 // combined to create the final output CODE section.
 class InputFunction : public InputChunk {
 public:
   InputFunction(const WasmSignature &S, const WasmFunction *Func,
                 const ObjFile *F)
-      : InputChunk(F), Signature(S), Function(Func) {}
+      : InputChunk(F, InputChunk::Function), Signature(S), Function(Func) {}
+
+  static bool classof(const InputChunk *C) { return C->kind() == InputChunk::Function; }
 
   virtual StringRef getName() const { return Function->Name; }
   StringRef getComdat() const override { return Function->Comdat; }
   uint32_t getOutputIndex() const { return OutputIndex.getValue(); }
   bool hasOutputIndex() const { return OutputIndex.hasValue(); }
   void setOutputIndex(uint32_t Index);
   uint32_t getTableIndex() const { return TableIndex.getValue(); }
   bool hasTableIndex() const { return TableIndex.hasValue(); }

wasm/InputFiles.h

 
 private:
   uint32_t relocateVirtualAddress(uint32_t Index) const;
   uint32_t relocateTypeIndex(uint32_t Original) const;
   uint32_t relocateGlobalIndex(uint32_t Original) const;
   uint32_t relocateTableIndex(uint32_t Original) const;
 
   Symbol *createDefined(const WasmSymbol &Sym, Symbol::Kind Kind,
-                        const InputSegment *Segment = nullptr,
-                        InputFunction *Function = nullptr,
+                        InputChunk *Chunk = nullptr,
                         uint32_t Address = UINT32_MAX);
   Symbol *createUndefined(const WasmSymbol &Sym, Symbol::Kind Kind,
                           const WasmSignature *Signature = nullptr);
   void initializeSymbols();
   InputSegment *getSegment(const WasmSymbol &WasmSym) const;
   const WasmSignature *getFunctionSig(const WasmSymbol &Sym) const;
   uint32_t getGlobalValue(const WasmSymbol &Sym) const;
   InputFunction *getFunction(const WasmSymbol &Sym) const;

wasm/InputFiles.cpp

void ObjFile::initializeSymbols() {
   // in the object
   for (const SymbolRef &Sym : WasmObj->symbols()) {
     const WasmSymbol &WasmSym = WasmObj->getWasmSymbol(Sym.getRawDataRefImpl());
     Symbol *S;
     switch (WasmSym.Type) {
     case WasmSymbol::SymbolType::FUNCTION_EXPORT: {
       InputFunction *Function = getFunction(WasmSym);
       if (!isExcludedByComdat(Function)) {
-        S = createDefined(WasmSym, Symbol::Kind::DefinedFunctionKind, nullptr,
-                          Function);
+        S = createDefined(WasmSym, Symbol::Kind::DefinedFunctionKind, Function);
         break;
       } else {
         Function->Discarded = true;
         LLVM_FALLTHROUGH; // Exclude function, and add the symbol as undefined
       }
     }
     case WasmSymbol::SymbolType::FUNCTION_IMPORT:
       S = createUndefined(WasmSym, Symbol::Kind::UndefinedFunctionKind,
                           getFunctionSig(WasmSym));
       break;
     case WasmSymbol::SymbolType::GLOBAL_EXPORT: {
       InputSegment *Segment = getSegment(WasmSym);
       if (!isExcludedByComdat(Segment)) {
         S = createDefined(WasmSym, Symbol::Kind::DefinedGlobalKind,
-                          Segment, nullptr, getGlobalValue(WasmSym));
+                          Segment, getGlobalValue(WasmSym));
         break;
       } else {
         Segment->Discarded = true;
         LLVM_FALLTHROUGH; // Exclude global, and add the symbol as undefined
       }
     }
     case WasmSymbol::SymbolType::GLOBAL_IMPORT:
       S = createUndefined(WasmSym, Symbol::Kind::UndefinedGlobalKind);

 }
 
 Symbol *ObjFile::createUndefined(const WasmSymbol &Sym, Symbol::Kind Kind,
                                  const WasmSignature *Signature) {
   return Symtab->addUndefined(Sym.Name, Kind, Sym.Flags, this, Signature);
 }
 
 Symbol *ObjFile::createDefined(const WasmSymbol &Sym, Symbol::Kind Kind,
-                               const InputSegment *Segment,
-                               InputFunction *Function, uint32_t Address) {
+                               InputChunk *Chunk, uint32_t Address) {
   Symbol *S;
   if (Sym.isLocal()) {
     S = make<Symbol>(Sym.Name, true);
-    S->update(Kind, this, Sym.Flags, Segment, Function, Address);
+    S->update(Kind, this, Sym.Flags, Chunk, Address);
     return S;
   }
-  return Symtab->addDefined(Sym.Name, Kind, Sym.Flags, this, Segment, Function,
+  return Symtab->addDefined(Sym.Name, Kind, Sym.Flags, this, Chunk,
                             Address);
 }
 
 void ArchiveFile::parse() {
   // Parse a MemoryBufferRef as an archive file.
   DEBUG(dbgs() << "Parsing library: " << toString(this) << "\n");
   File = CHECK(Archive::create(MB), toString(this));
 

wasm/SymbolTable.h

public:
   void reportDuplicate(Symbol *Existing, InputFile *NewFile);
   void reportRemainingUndefines();
 
   ArrayRef<Symbol *> getSymbols() const { return SymVector; }
   Symbol *find(StringRef Name);
   ObjFile *findComdat(StringRef Name) const;
 
   Symbol *addDefined(StringRef Name, Symbol::Kind Kind, uint32_t Flags,
-                     InputFile *F, const InputSegment *Segment = nullptr,
-                     InputFunction *Function = nullptr, uint32_t Address = 0);
+                     InputFile *F,
+                     InputChunk *Chunk = nullptr, uint32_t Address = 0);
   Symbol *addUndefined(StringRef Name, Symbol::Kind Kind, uint32_t Flags,
                        InputFile *F, const WasmSignature *Signature = nullptr);
   Symbol *addUndefinedFunction(StringRef Name, const WasmSignature *Type);
   Symbol *addDefinedGlobal(StringRef Name);
   Symbol *addDefinedFunction(StringRef Name, const WasmSignature *Type,
                              uint32_t Flags);
   void addLazy(ArchiveFile *F, const Archive::Symbol *Sym);
   bool addComdat(StringRef Name, ObjFile *);

wasm/SymbolTable.cpp

if (*NewSig == OldSig)
     return;
 
   error("function signature mismatch: " + Existing.getName() +
         "\n>>> defined as " + toString(OldSig) + " in " +
         toString(Existing.getFile()) + "\n>>> defined as " + toString(*NewSig) +
         " in " + F.getName());
 }
 
+static void checkSymbolTypes(const Symbol &Existing, const InputFile &F,
+                             Symbol::Kind Kind, const InputChunk *Chunk) {
+  const WasmSignature* Sig = nullptr;
+  if (auto* F = dyn_cast_or_null<InputFunction>(Chunk))
+    Sig = &F->Signature;
+  return checkSymbolTypes(Existing, F, Kind, Sig);
+}
+
 Symbol *SymbolTable::addDefinedFunction(StringRef Name,
                                         const WasmSignature *Type,
                                         uint32_t Flags) {
   DEBUG(dbgs() << "addDefinedFunction: " << Name << "\n");
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name);
   if (WasInserted) {
if (WasInserted) {
     DEBUG(dbgs() << "resolving existing undefined global: " << Name << "\n");
     S->update(Symbol::DefinedGlobalKind);
   }
   return S;
 }
 
 Symbol *SymbolTable::addDefined(StringRef Name, Symbol::Kind Kind,
                                 uint32_t Flags, InputFile *F,
-                                const InputSegment *Segment,
-                                InputFunction *Function, uint32_t Address) {
+                                InputChunk *Chunk, uint32_t Address) {
   DEBUG(dbgs() << "addDefined: " << Name << " addr:" << Address << "\n");
   Symbol *S;
   bool WasInserted;
 
   std::tie(S, WasInserted) = insert(Name);
   if (WasInserted) {
-    S->update(Kind, F, Flags, Segment, Function, Address);
+    S->update(Kind, F, Flags, Chunk, Address);
   } else if (S->isLazy()) {
     // The existing symbol is lazy. Replace it without checking types since
     // lazy symbols don't have any type information.
     DEBUG(dbgs() << "replacing existing lazy symbol: " << Name << "\n");
-    S->update(Kind, F, Flags, Segment, Function, Address);
+    S->update(Kind, F, Flags, Chunk, Address);
   } else if (!S->isDefined()) {
     // The existing symbol table entry is undefined. The new symbol replaces
     // it, after checking the type matches
     DEBUG(dbgs() << "resolving existing undefined symbol: " << Name << "\n");
-    checkSymbolTypes(*S, *F, Kind, Function ? &Function->Signature : nullptr);
-    S->update(Kind, F, Flags, Segment, Function, Address);
+    checkSymbolTypes(*S, *F, Kind, Chunk);
+    S->update(Kind, F, Flags, Chunk, Address);
   } else if ((Flags & WASM_SYMBOL_BINDING_MASK) == WASM_SYMBOL_BINDING_WEAK) {
     // the new symbol is weak we can ignore it
     DEBUG(dbgs() << "existing symbol takes precedence\n");
   } else if (S->isWeak()) {
     // the new symbol is not weak and the existing symbol is, so we replace
     // it
     DEBUG(dbgs() << "replacing existing weak symbol\n");
-    checkSymbolTypes(*S, *F, Kind, Function ? &Function->Signature : nullptr);
-    S->update(Kind, F, Flags, Segment, Function, Address);
+    checkSymbolTypes(*S, *F, Kind, Chunk);
+    S->update(Kind, F, Flags, Chunk, Address);
   } else {
     // neither symbol is week. They conflict.
     reportDuplicate(S, F);
   }
   return S;
 }
 
 Symbol *SymbolTable::addUndefinedFunction(StringRef Name,

wasm/Symbols.h

 
 using llvm::object::Archive;
 using llvm::wasm::WasmSignature;
 
 namespace lld {
 namespace wasm {
 
 class InputFile;
-class InputSegment;
-class InputFunction;
+class InputChunk;
 
 class Symbol {
 public:
   enum Kind {
     DefinedFunctionKind,
     DefinedGlobalKind,
 
     LazyKind,
public:
   bool isWeak() const;
   bool isHidden() const;
 
   // Returns the symbol name.
   StringRef getName() const { return Name; }
 
   // Returns the file from which this symbol was created.
   InputFile *getFile() const { return File; }
+  InputChunk *getChunk() const { return Chunk; }
 
   bool hasFunctionType() const { return FunctionType != nullptr; }
   const WasmSignature &getFunctionType() const;
   void setFunctionType(const WasmSignature *Type);
   void setHidden(bool IsHidden);
 
   uint32_t getOutputIndex() const;
 
public:
 
   // Returns the virtual address of a defined global.
   // Only works for globals, not functions.
   uint32_t getVirtualAddress() const;
 
   void setVirtualAddress(uint32_t VA);
 
   void update(Kind K, InputFile *F = nullptr, uint32_t Flags = 0,
-              const InputSegment *Segment = nullptr,
-              InputFunction *Function = nullptr, uint32_t Address = UINT32_MAX);
+              InputChunk *chunk = nullptr,
+              uint32_t Address = UINT32_MAX);
 
   void setArchiveSymbol(const Archive::Symbol &Sym) { ArchiveSymbol = Sym; }
   const Archive::Symbol &getArchiveSymbol() { return ArchiveSymbol; }
-  InputFunction *getFunction() { return Function; }
 
 protected:
   uint32_t Flags;
   uint32_t VirtualAddress = 0;
 
   StringRef Name;
   Archive::Symbol ArchiveSymbol = {nullptr, 0, 0};
   Kind SymbolKind = InvalidKind;
   InputFile *File = nullptr;
-  const InputSegment *Segment = nullptr;
-  InputFunction *Function = nullptr;
+  InputChunk *Chunk = nullptr;
   llvm::Optional<uint32_t> OutputIndex;
   llvm::Optional<uint32_t> TableIndex;
   const WasmSignature *FunctionType = nullptr;
 };
 
 } // namespace wasm
 
 // Returns a symbol name for an error message.
 std::string toString(const wasm::Symbol &Sym);
 std::string toString(wasm::Symbol::Kind Kind);
 
 } // namespace lld
 
 #endif

wasm/Symbols.cpp

 #define DEBUG_TYPE "lld"
 
 using namespace llvm;
 using namespace llvm::wasm;
 using namespace lld;
 using namespace lld::wasm;
 
 const WasmSignature &Symbol::getFunctionType() const {
-  if (Function != nullptr)
-    return Function->Signature;
+  if (Chunk != nullptr)
+    return dyn_cast<InputFunction>(Chunk)->Signature;
 
   assert(FunctionType != nullptr);
   return *FunctionType;
 }
 
 void Symbol::setFunctionType(const WasmSignature *Type) {
   assert(FunctionType == nullptr);
-  assert(Function == nullptr);
+  assert(!Chunk);
   FunctionType = Type;
 }
 
 uint32_t Symbol::getVirtualAddress() const {
   assert(isGlobal());
   DEBUG(dbgs() << "getVirtualAddress: " << getName() << "\n");
-  return Segment ? Segment->translateVA(VirtualAddress) : VirtualAddress;
+  return Chunk ? dyn_cast<InputSegment>(Chunk)->translateVA(VirtualAddress) : VirtualAddress;
 }
 
 bool Symbol::hasOutputIndex() const {
-  if (Function)
-    return Function->hasOutputIndex();
+  if (auto* F = dyn_cast_or_null<InputFunction>(Chunk))
+    return F->hasOutputIndex();
   return OutputIndex.hasValue();
 }
 
 uint32_t Symbol::getOutputIndex() const {
-  if (Function)
-    return Function->getOutputIndex();
+  if (auto* F = dyn_cast_or_null<InputFunction>(Chunk))
+    return F->getOutputIndex();
   return OutputIndex.getValue();
 }
 
 void Symbol::setVirtualAddress(uint32_t Value) {
   DEBUG(dbgs() << "setVirtualAddress " << Name << " -> " << Value << "\n");
   assert(isGlobal());
   VirtualAddress = Value;
 }
 
 void Symbol::setOutputIndex(uint32_t Index) {
   DEBUG(dbgs() << "setOutputIndex " << Name << " -> " << Index << "\n");
-  assert(!Function);
+  assert(!dyn_cast_or_null<InputFunction>(Chunk));
   assert(!OutputIndex.hasValue());
   OutputIndex = Index;
 }
 
 uint32_t Symbol::getTableIndex() const {
-  if (Function)
-    return Function->getTableIndex();
+  if (auto* F = dyn_cast_or_null<InputFunction>(Chunk))
+    return F->getTableIndex();
   return TableIndex.getValue();
 }
 
 bool Symbol::hasTableIndex() const {
-  if (Function)
-    return Function->hasTableIndex();
+  if (auto* F = dyn_cast_or_null<InputFunction>(Chunk))
+    return F->hasTableIndex();
   return TableIndex.hasValue();
 }
 
 void Symbol::setTableIndex(uint32_t Index) {
   // For imports, we set the table index here on the Symbol; for defined
   // functions we set the index on the InputFunction so that we don't export
   // the same thing twice (keeps the table size down).
-  if (Function) {
-    Function->setTableIndex(Index);
+  if (auto* F = dyn_cast_or_null<InputFunction>(Chunk)) {
+    F->setTableIndex(Index);
     return;
   }
   DEBUG(dbgs() << "setTableIndex " << Name << " -> " << Index << "\n");
   assert(!TableIndex.hasValue());
   TableIndex = Index;
 }
 
 void Symbol::update(Kind K, InputFile *F, uint32_t Flags_,
-                    const InputSegment *Seg, InputFunction *Func,
+                    InputChunk *Chunk_,
                     uint32_t Address) {
   SymbolKind = K;
   File = F;
   Flags = Flags_;
-  Segment = Seg;
-  Function = Func;
+  Chunk = Chunk_;
   if (Address != UINT32_MAX)
     setVirtualAddress(Address);
 }
 
 bool Symbol::isWeak() const {
   return (Flags & WASM_SYMBOL_BINDING_MASK) == WASM_SYMBOL_BINDING_WEAK;
 }
 

wasm/Writer.cpp

ExportedSymbols.emplace_back(
         WasmExportEntry{Config->CtorSymbol, Config->CtorSymbol->getName()});
 
   for (ObjFile *File : Symtab->ObjectFiles) {
     for (Symbol *Sym : File->getSymbols()) {
       if (!Sym->isDefined() || File != Sym->getFile())
         continue;
       if (Sym->isGlobal())
         continue;
-      if (Sym->getFunction()->Discarded)
+      if (Sym->getChunk()->Discarded)
         continue;
 
       if ((Sym->isHidden() || Sym->isLocal()) && !ExportHidden)
         continue;
       ExportedSymbols.emplace_back(WasmExportEntry{Sym, BudgeLocalName(Sym)});
     }
   }