[lld] r320428 - [WebAssembly] De-dup indirect function table.

Sam Clegg via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 11 14:00:57 PST 2017


Author: sbc
Date: Mon Dec 11 14:00:56 2017
New Revision: 320428

URL: http://llvm.org/viewvc/llvm-project?rev=320428&view=rev
Log:
[WebAssembly] De-dup indirect function table.

Create the indirect function table based on symbols rather
than just duplicating the input entries.  This has the
effect of de-duplicating the table.

This is a followup to the equivalent change made for globals:
  https://reviews.llvm.org/D40859

Partially based on a patch by Nicholas Wilson:
  https://reviews.llvm.org/D40845

Differential Revision: https://reviews.llvm.org/D40989

Modified:
    lld/trunk/test/wasm/call-indirect.ll
    lld/trunk/test/wasm/weak-symbols.ll
    lld/trunk/wasm/InputFiles.cpp
    lld/trunk/wasm/InputFiles.h
    lld/trunk/wasm/Symbols.cpp
    lld/trunk/wasm/Symbols.h
    lld/trunk/wasm/Writer.cpp

Modified: lld/trunk/test/wasm/call-indirect.ll
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/wasm/call-indirect.ll?rev=320428&r1=320427&r2=320428&view=diff
==============================================================================
--- lld/trunk/test/wasm/call-indirect.ll (original)
+++ lld/trunk/test/wasm/call-indirect.ll Mon Dec 11 14:00:56 2017
@@ -43,8 +43,8 @@ entry:
 ; CHECK-NEXT:       - ElemType:        ANYFUNC
 ; CHECK-NEXT:         Limits:          
 ; CHECK-NEXT:           Flags:           0x00000001
-; CHECK-NEXT:           Initial:         0x00000004
-; CHECK-NEXT:           Maximum:         0x00000004
+; CHECK-NEXT:           Initial:         0x00000003
+; CHECK-NEXT:           Maximum:         0x00000003
 ; CHECK-NEXT:   - Type:            MEMORY
 ; CHECK-NEXT:     Memories:        
 ; CHECK-NEXT:       - Initial:         0x00000002
@@ -77,7 +77,7 @@ entry:
 ; CHECK-NEXT:       - Offset:          
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           1
-; CHECK-NEXT:         Functions:       [ 0, 2, 2 ]
+; CHECK-NEXT:         Functions:       [ 0, 2 ]
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Functions:       
 ; CHECK:            - Locals:          
@@ -90,7 +90,7 @@ entry:
 ; CHECK-NEXT:         Offset:          
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           1024
-; CHECK-NEXT:         Content:         '010000000200000003000000'
+; CHECK-NEXT:         Content:         '010000000200000002000000'
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        12

Modified: lld/trunk/test/wasm/weak-symbols.ll
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/test/wasm/weak-symbols.ll?rev=320428&r1=320427&r2=320428&view=diff
==============================================================================
--- lld/trunk/test/wasm/weak-symbols.ll (original)
+++ lld/trunk/test/wasm/weak-symbols.ll Mon Dec 11 14:00:56 2017
@@ -31,8 +31,8 @@ entry:
 ; CHECK-NEXT:       - ElemType:        ANYFUNC
 ; CHECK-NEXT:         Limits:          
 ; CHECK-NEXT:           Flags:           0x00000001
-; CHECK-NEXT:           Initial:         0x00000003
-; CHECK-NEXT:           Maximum:         0x00000003
+; CHECK-NEXT:           Initial:         0x00000002
+; CHECK-NEXT:           Maximum:         0x00000002
 ; CHECK-NEXT:   - Type:            MEMORY
 ; CHECK-NEXT:     Memories:        
 ; CHECK-NEXT:       - Initial:         0x00000002
@@ -65,7 +65,7 @@ entry:
 ; CHECK-NEXT:       - Offset:          
 ; CHECK-NEXT:           Opcode:          I32_CONST
 ; CHECK-NEXT:           Value:           1
-; CHECK-NEXT:         Functions:       [ 1, 1 ]
+; CHECK-NEXT:         Functions:       [ 1 ]
 ; CHECK-NEXT:   - Type:            CODE
 ; CHECK-NEXT:     Functions:       
 ; CHECK-NEXT:       - Locals:          
@@ -77,7 +77,7 @@ entry:
 ; CHECK-NEXT:       - Locals:          
 ; CHECK-NEXT:         Body:            41020B
 ; CHECK-NEXT:       - Locals:          
-; CHECK-NEXT:         Body:            4182808080000B
+; CHECK-NEXT:         Body:            4181808080000B
 ; CHECK-NEXT:   - Type:            CUSTOM
 ; CHECK-NEXT:     Name:            linking
 ; CHECK-NEXT:     DataSize:        0

Modified: lld/trunk/wasm/InputFiles.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/wasm/InputFiles.cpp?rev=320428&r1=320427&r2=320428&view=diff
==============================================================================
--- lld/trunk/wasm/InputFiles.cpp (original)
+++ lld/trunk/wasm/InputFiles.cpp Mon Dec 11 14:00:56 2017
@@ -46,7 +46,6 @@ void ObjFile::dumpInfo() const {
   log("reloc info for: " + getName() + "\n" +
       "        FunctionIndexOffset : " + Twine(FunctionIndexOffset) + "\n" +
       "         NumFunctionImports : " + Twine(NumFunctionImports()) + "\n" +
-      "           TableIndexOffset : " + Twine(TableIndexOffset) + "\n" +
       "           NumGlobalImports : " + Twine(NumGlobalImports()) + "\n");
 }
 
@@ -54,11 +53,15 @@ bool ObjFile::isImportedFunction(uint32_
   return Index < NumFunctionImports();
 }
 
-const Symbol *ObjFile::getFunctionSymbol(uint32_t Index) const {
+Symbol *ObjFile::getFunctionSymbol(uint32_t Index) const {
   return FunctionSymbols[Index];
 }
 
-const Symbol *ObjFile::getGlobalSymbol(uint32_t Index) const {
+Symbol *ObjFile::getTableSymbol(uint32_t Index) const {
+  return TableSymbols[Index];
+}
+
+Symbol *ObjFile::getGlobalSymbol(uint32_t Index) const {
   return GlobalSymbols[Index];
 }
 
@@ -67,7 +70,7 @@ uint32_t ObjFile::getRelocatedAddress(ui
 }
 
 uint32_t ObjFile::relocateFunctionIndex(uint32_t Original) const {
-  const Symbol *Sym = getFunctionSymbol(Original);
+  Symbol *Sym = getFunctionSymbol(Original);
   uint32_t Index = Sym->getOutputIndex();
   DEBUG(dbgs() << "relocateFunctionIndex: " << toString(*Sym) << ": "
                << Original << " -> " << Index << "\n");
@@ -79,11 +82,15 @@ uint32_t ObjFile::relocateTypeIndex(uint
 }
 
 uint32_t ObjFile::relocateTableIndex(uint32_t Original) const {
-  return Original + TableIndexOffset;
+  Symbol *Sym = getTableSymbol(Original);
+  uint32_t Index = Sym->getTableIndex();
+  DEBUG(dbgs() << "relocateTableIndex: " << toString(*Sym) << ": " << Original
+               << " -> " << Index << "\n");
+  return Index;
 }
 
 uint32_t ObjFile::relocateGlobalIndex(uint32_t Original) const {
-  const Symbol *Sym = getGlobalSymbol(Original);
+  Symbol *Sym = getGlobalSymbol(Original);
   uint32_t Index = Sym->getOutputIndex();
   DEBUG(dbgs() << "relocateGlobalIndex: " << toString(*Sym) << ": " << Original
                << " -> " << Index << "\n");
@@ -152,9 +159,11 @@ void ObjFile::initializeSymbols() {
   for (const WasmSegment &Seg : WasmObj->dataSegments())
     Segments.emplace_back(make<InputSegment>(&Seg, this));
 
-  Symbol *S;
+  // Populate `FunctionSymbols` and `GlobalSymbols` based on the WasmSymbols
+  // 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_IMPORT:
     case WasmSymbol::SymbolType::GLOBAL_IMPORT:
@@ -183,8 +192,26 @@ void ObjFile::initializeSymbols() {
     }
   }
 
-  DEBUG(dbgs() << "Functions: " << FunctionSymbols.size() << "\n");
-  DEBUG(dbgs() << "Globals  : " << GlobalSymbols.size() << "\n");
+  // Populate `TableSymbols` with all symbols that are called indirectly
+  uint32_t SegmentCount = WasmObj->elements().size();
+  if (SegmentCount) {
+    if (SegmentCount > 1)
+      fatal(getName() + ": contains more than one element segment");
+    const WasmElemSegment &Segment = WasmObj->elements()[0];
+    if (Segment.Offset.Opcode != WASM_OPCODE_I32_CONST)
+      fatal(getName() + ": unsupported element segment");
+    if (Segment.TableIndex != 0)
+      fatal(getName() + ": unsupported table index in elem segment");
+    if (Segment.Offset.Value.Int32 != 0)
+      fatal(getName() + ": unsupported element segment offset");
+    TableSymbols.reserve(Segment.Functions.size());
+    for (uint64_t FunctionIndex : Segment.Functions)
+      TableSymbols.push_back(getFunctionSymbol(FunctionIndex));
+  }
+
+  DEBUG(dbgs() << "TableSymbols: " << TableSymbols.size() << "\n");
+  DEBUG(dbgs() << "Functions   : " << FunctionSymbols.size() << "\n");
+  DEBUG(dbgs() << "Globals     : " << GlobalSymbols.size() << "\n");
 }
 
 Symbol *ObjFile::createUndefined(const WasmSymbol &Sym) {

Modified: lld/trunk/wasm/InputFiles.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/wasm/InputFiles.h?rev=320428&r1=320427&r2=320428&view=diff
==============================================================================
--- lld/trunk/wasm/InputFiles.h (original)
+++ lld/trunk/wasm/InputFiles.h Mon Dec 11 14:00:56 2017
@@ -103,7 +103,6 @@ public:
   size_t NumGlobalImports() const { return GlobalImports; }
 
   int32_t FunctionIndexOffset = 0;
-  int32_t TableIndexOffset = 0;
   const WasmSection *CodeSection = nullptr;
   std::vector<OutputRelocation> CodeRelocations;
   int32_t CodeOffset = 0;
@@ -113,6 +112,7 @@ public:
   std::vector<InputSegment *> Segments;
 
   ArrayRef<Symbol *> getSymbols() { return Symbols; }
+  ArrayRef<Symbol *> getTableSymbols() { return TableSymbols; }
 
 private:
   Symbol *createDefined(const WasmSymbol &Sym,
@@ -120,17 +120,21 @@ private:
   Symbol *createUndefined(const WasmSymbol &Sym);
   void initializeSymbols();
   InputSegment *getSegment(const WasmSymbol &WasmSym);
-  const Symbol *getFunctionSymbol(uint32_t Index) const;
-  const Symbol *getGlobalSymbol(uint32_t Index) const;
+  Symbol *getFunctionSymbol(uint32_t FunctionIndex) const;
+  Symbol *getTableSymbol(uint32_t TableIndex) const;
+  Symbol *getGlobalSymbol(uint32_t GlobalIndex) const;
 
   // List of all symbols referenced or defined by this file.
   std::vector<Symbol *> Symbols;
 
   // List of all function symbols indexed by the function index space
-  std::vector<const Symbol *> FunctionSymbols;
+  std::vector<Symbol *> FunctionSymbols;
 
   // List of all global symbols indexed by the global index space
-  std::vector<const Symbol *> GlobalSymbols;
+  std::vector<Symbol *> GlobalSymbols;
+
+  // List of all indirect symbols indexed by table index space.
+  std::vector<Symbol *> TableSymbols;
 
   uint32_t GlobalImports = 0;
   uint32_t FunctionImports = 0;

Modified: lld/trunk/wasm/Symbols.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/wasm/Symbols.cpp?rev=320428&r1=320427&r2=320428&view=diff
==============================================================================
--- lld/trunk/wasm/Symbols.cpp (original)
+++ lld/trunk/wasm/Symbols.cpp Mon Dec 11 14:00:56 2017
@@ -67,10 +67,16 @@ void Symbol::setVirtualAddress(uint32_t
 
 void Symbol::setOutputIndex(uint32_t Index) {
   DEBUG(dbgs() << "setOutputIndex " << Name << " -> " << Index << "\n");
-  assert(!hasOutputIndex());
+  assert(!OutputIndex.hasValue());
   OutputIndex = Index;
 }
 
+void Symbol::setTableIndex(uint32_t Index) {
+  DEBUG(dbgs() << "setTableIndex " << Name << " -> " << Index << "\n");
+  assert(!TableIndex.hasValue());
+  TableIndex = Index;
+}
+
 void Symbol::update(Kind K, InputFile *F, const WasmSymbol *WasmSym,
                     const InputSegment *Seg, const WasmSignature *Sig) {
   SymbolKind = K;

Modified: lld/trunk/wasm/Symbols.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/wasm/Symbols.h?rev=320428&r1=320427&r2=320428&view=diff
==============================================================================
--- lld/trunk/wasm/Symbols.h (original)
+++ lld/trunk/wasm/Symbols.h Mon Dec 11 14:00:56 2017
@@ -72,6 +72,7 @@ public:
   bool hasFunctionType() const { return FunctionType != nullptr; }
   const WasmSignature &getFunctionType() const;
   uint32_t getOutputIndex() const;
+  uint32_t getTableIndex() const { return TableIndex.getValue(); }
 
   // Returns the virtual address of a defined global.
   // Only works for globals, not functions.
@@ -84,6 +85,12 @@ public:
   // space of the output object.
   void setOutputIndex(uint32_t Index);
 
+  // Returns true if a table index has been set for this symbol
+  bool hasTableIndex() const { return TableIndex.hasValue(); }
+
+  // Set the table index of the symbol
+  void setTableIndex(uint32_t Index);
+
   void setVirtualAddress(uint32_t VA);
 
   void update(Kind K, InputFile *F = nullptr, const WasmSymbol *Sym = nullptr,
@@ -108,6 +115,7 @@ protected:
   const WasmSymbol *Sym = nullptr;
   const InputSegment *Segment = nullptr;
   llvm::Optional<uint32_t> OutputIndex;
+  llvm::Optional<uint32_t> TableIndex;
   llvm::Optional<uint32_t> VirtualAddress;
   const WasmSignature *FunctionType;
 };

Modified: lld/trunk/wasm/Writer.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/wasm/Writer.cpp?rev=320428&r1=320427&r2=320428&view=diff
==============================================================================
--- lld/trunk/wasm/Writer.cpp (original)
+++ lld/trunk/wasm/Writer.cpp Mon Dec 11 14:00:56 2017
@@ -103,10 +103,7 @@ private:
   uint64_t FileSize = 0;
   uint32_t DataSize = 0;
   uint32_t NumFunctions = 0;
-  uint32_t NumGlobals = 0;
   uint32_t NumMemoryPages = 0;
-  uint32_t NumTableElems = 0;
-  uint32_t NumElements = 0;
   uint32_t InitialTableOffset = 0;
 
   std::vector<const WasmSignature *> Types;
@@ -114,6 +111,7 @@ private:
   std::vector<const Symbol *> FunctionImports;
   std::vector<const Symbol *> GlobalImports;
   std::vector<const Symbol *> DefinedGlobals;
+  std::vector<const Symbol *> IndirectFunctions;
 
   // Elements that are used to construct the final output
   std::string Header;
@@ -236,14 +234,24 @@ void Writer::createGlobalSection() {
 }
 
 void Writer::createTableSection() {
+  // Always output a table section, even if there are no indirect calls.
+  // There are two reasons for this:
+  //  1. For executables it is useful to have an empty table slot at 0
+  //     which can be filled with a null function call handler.
+  //  2. If we don't do this, any program that contains a call_indirect but
+  //     no address-taken function will fail at validation time since it is
+  //     a validation error to include a call_indirect instruction if there
+  //     is not table.
+  uint32_t TableSize = InitialTableOffset + IndirectFunctions.size();
+
   SyntheticSection *Section = createSyntheticSection(WASM_SEC_TABLE);
   raw_ostream &OS = Section->getStream();
 
   writeUleb128(OS, 1, "table count");
   writeSleb128(OS, WASM_TYPE_ANYFUNC, "table type");
   writeUleb128(OS, WASM_LIMITS_FLAG_HAS_MAX, "table flags");
-  writeUleb128(OS, NumTableElems, "table initial size");
-  writeUleb128(OS, NumTableElems, "table max size");
+  writeUleb128(OS, TableSize, "table initial size");
+  writeUleb128(OS, TableSize, "table max size");
 }
 
 void Writer::createExportSection() {
@@ -309,7 +317,7 @@ void Writer::createExportSection() {
 void Writer::createStartSection() {}
 
 void Writer::createElemSection() {
-  if (!NumElements)
+  if (IndirectFunctions.empty())
     return;
 
   SyntheticSection *Section = createSyntheticSection(WASM_SEC_ELEM);
@@ -321,13 +329,14 @@ void Writer::createElemSection() {
   InitExpr.Opcode = WASM_OPCODE_I32_CONST;
   InitExpr.Value.Int32 = InitialTableOffset;
   writeInitExpr(OS, InitExpr);
-  writeUleb128(OS, NumElements, "elem count");
+  writeUleb128(OS, IndirectFunctions.size(), "elem count");
 
-  for (ObjFile *File : Symtab->ObjectFiles)
-    for (const WasmElemSegment &Segment : File->getWasmObj()->elements())
-      for (uint64_t FunctionIndex : Segment.Functions)
-        writeUleb128(OS, File->relocateFunctionIndex(FunctionIndex),
-                     "function index");
+  uint32_t TableIndex = InitialTableOffset;
+  for (const Symbol *Sym : IndirectFunctions) {
+    assert(Sym->getTableIndex() == TableIndex);
+    writeUleb128(OS, Sym->getOutputIndex(), "function index");
+    ++TableIndex;
+  }
 }
 
 void Writer::createCodeSection() {
@@ -526,8 +535,6 @@ void Writer::createSections() {
 }
 
 void Writer::calculateOffsets() {
-  NumTableElems = InitialTableOffset;
-
   for (ObjFile *File : Symtab->ObjectFiles) {
     const WasmObjectFile *WasmFile = File->getWasmObj();
 
@@ -537,34 +544,8 @@ void Writer::calculateOffsets() {
     NumFunctions += WasmFile->functions().size();
 
     // Memory
-    if (WasmFile->memories().size()) {
-      if (WasmFile->memories().size() > 1) {
-        fatal(File->getName() + ": contains more than one memory");
-      }
-    }
-
-    // Table
-    uint32_t TableCount = WasmFile->tables().size();
-    if (TableCount) {
-      if (TableCount > 1)
-        fatal(File->getName() + ": contains more than one table");
-      File->TableIndexOffset = NumTableElems;
-      NumTableElems += WasmFile->tables()[0].Limits.Initial;
-    }
-
-    // Elem
-    uint32_t SegmentCount = WasmFile->elements().size();
-    if (SegmentCount) {
-      if (SegmentCount > 1)
-        fatal(File->getName() + ": contains more than element segment");
-
-      const WasmElemSegment &Segment = WasmFile->elements()[0];
-      if (Segment.TableIndex != 0)
-        fatal(File->getName() + ": unsupported table index");
-      if (Segment.Offset.Value.Int32 != 0)
-        fatal(File->getName() + ": unsupported segment offset");
-      NumElements += Segment.Functions.size();
-    }
+    if (WasmFile->memories().size() > 1)
+      fatal(File->getName() + ": contains more than one memory");
   }
 }
 
@@ -611,8 +592,11 @@ void Writer::assignSymbolIndexes() {
   if (Config->EmitRelocs)
     DefinedGlobals.reserve(Symtab->getSymbols().size());
 
+  uint32_t TableIndex = InitialTableOffset;
+
   for (ObjFile *File : Symtab->ObjectFiles) {
     DEBUG(dbgs() << "assignSymbolIndexes: " << File->getName() << "\n");
+
     for (Symbol *Sym : File->getSymbols()) {
       // Assign indexes for symbols defined with this file.
       if (!Sym->isDefined() || File != Sym->getFile())
@@ -626,6 +610,13 @@ void Writer::assignSymbolIndexes() {
         Sym->setOutputIndex(GlobalIndex++);
       }
     }
+
+    for (Symbol *Sym : File->getTableSymbols()) {
+      if (!Sym->hasTableIndex()) {
+        Sym->setTableIndex(TableIndex++);
+        IndirectFunctions.emplace_back(Sym);
+      }
+    }
   }
 }
 
@@ -679,12 +670,12 @@ void Writer::run() {
   calculateOffsets();
 
   if (errorHandler().Verbose) {
-    log("NumFunctions    : " + Twine(NumFunctions));
-    log("NumGlobals      : " + Twine(NumGlobals));
-    log("NumImports      : " +
+    log("Defined Functions: " + Twine(NumFunctions));
+    log("Defined Globals  : " + Twine(DefinedGlobals.size()));
+    log("Function Imports : " + Twine(FunctionImports.size()));
+    log("Global Imports   : " + Twine(GlobalImports.size()));
+    log("Total Imports    : " +
         Twine(FunctionImports.size() + GlobalImports.size()));
-    log("FunctionImports : " + Twine(FunctionImports.size()));
-    log("GlobalImports   : " + Twine(GlobalImports.size()));
     for (ObjFile *File : Symtab->ObjectFiles)
       File->dumpInfo();
   }




More information about the llvm-commits mailing list