[lld] [llvm] [Object][Wasm] Move WasmSymbolInfo directly into WasmSymbol (NFC) (PR #80219)

Derek Schuff via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 2 10:41:13 PST 2024


https://github.com/dschuff updated https://github.com/llvm/llvm-project/pull/80219

>From 29e818765e6e2ea6ed0a14ef19cfd278dd631e9d Mon Sep 17 00:00:00 2001
From: Derek Schuff <dschuff at chromium.org>
Date: Wed, 31 Jan 2024 15:49:22 -0800
Subject: [PATCH 1/5] [Object][Wasm] Move WasmSymbolInfo directly into
 WasmSymbol (NFC)

---
 lld/wasm/InputFiles.cpp               | 20 ++++++++++----------
 llvm/include/llvm/BinaryFormat/Wasm.h |  1 -
 llvm/include/llvm/Object/Wasm.h       |  4 ++--
 llvm/lib/Object/WasmObjectFile.cpp    |  9 ++-------
 llvm/tools/obj2yaml/wasm2yaml.cpp     |  3 ++-
 5 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp
index f43c39b218787..394ca98d1265a 100644
--- a/lld/wasm/InputFiles.cpp
+++ b/lld/wasm/InputFiles.cpp
@@ -322,20 +322,20 @@ void ObjFile::addLegacyIndirectFunctionTableIfNeeded(
     return;
   }
 
-  auto *info = make<WasmSymbolInfo>();
-  info->Name = tableImport->Field;
-  info->Kind = WASM_SYMBOL_TYPE_TABLE;
-  info->ImportModule = tableImport->Module;
-  info->ImportName = tableImport->Field;
-  info->Flags = WASM_SYMBOL_UNDEFINED;
-  info->Flags |= WASM_SYMBOL_NO_STRIP;
-  info->ElementIndex = 0;
-  LLVM_DEBUG(dbgs() << "Synthesizing symbol for table import: " << info->Name
+  WasmSymbolInfo info;
+  info.Name = tableImport->Field;
+  info.Kind = WASM_SYMBOL_TYPE_TABLE;
+  info.ImportModule = tableImport->Module;
+  info.ImportName = tableImport->Field;
+  info.Flags = WASM_SYMBOL_UNDEFINED;
+  info.Flags |= WASM_SYMBOL_NO_STRIP;
+  info.ElementIndex = 0;
+  LLVM_DEBUG(dbgs() << "Synthesizing symbol for table import: " << info.Name
                     << "\n");
   const WasmGlobalType *globalType = nullptr;
   const WasmSignature *signature = nullptr;
   auto *wasmSym =
-      make<WasmSymbol>(*info, globalType, &tableImport->Table, signature);
+      make<WasmSymbol>(std::move(info), globalType, &tableImport->Table, signature);
   Symbol *sym = createUndefined(*wasmSym, false);
   // We're only sure it's a TableSymbol if the createUndefined succeeded.
   if (errorCount())
diff --git a/llvm/include/llvm/BinaryFormat/Wasm.h b/llvm/include/llvm/BinaryFormat/Wasm.h
index cd13b7014bfe2..9c1cf68d939a9 100644
--- a/llvm/include/llvm/BinaryFormat/Wasm.h
+++ b/llvm/include/llvm/BinaryFormat/Wasm.h
@@ -471,7 +471,6 @@ struct WasmLinkingData {
   uint32_t Version;
   std::vector<WasmInitFunc> InitFunctions;
   std::vector<StringRef> Comdats;
-  std::vector<WasmSymbolInfo> SymbolTable;
 };
 
 struct WasmSignature {
diff --git a/llvm/include/llvm/Object/Wasm.h b/llvm/include/llvm/Object/Wasm.h
index 927dce882f6ae..1a222e58ef906 100644
--- a/llvm/include/llvm/Object/Wasm.h
+++ b/llvm/include/llvm/Object/Wasm.h
@@ -34,7 +34,7 @@ namespace object {
 
 class WasmSymbol {
 public:
-  WasmSymbol(const wasm::WasmSymbolInfo &Info,
+  WasmSymbol(const wasm::WasmSymbolInfo &&Info,
              const wasm::WasmGlobalType *GlobalType,
              const wasm::WasmTableType *TableType,
              const wasm::WasmSignature *Signature)
@@ -43,7 +43,7 @@ class WasmSymbol {
     assert(!Signature || Signature->Kind != wasm::WasmSignature::Placeholder);
   }
 
-  const wasm::WasmSymbolInfo &Info;
+  const wasm::WasmSymbolInfo Info;
   const wasm::WasmGlobalType *GlobalType;
   const wasm::WasmTableType *TableType;
   const wasm::WasmSignature *Signature;
diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index 953e7c70b5f6e..636ec484e2d05 100644
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -642,9 +642,7 @@ Error WasmObjectFile::parseLinkingSectionSymtab(ReadContext &Ctx) {
   uint32_t Count = readVaruint32(Ctx);
   // Clear out any symbol information that was derived from the exports
   // section.
-  LinkingData.SymbolTable.clear();
   Symbols.clear();
-  LinkingData.SymbolTable.reserve(Count);
   Symbols.reserve(Count);
   StringSet<> SymbolNames;
 
@@ -844,8 +842,7 @@ Error WasmObjectFile::parseLinkingSectionSymtab(ReadContext &Ctx) {
       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(), GlobalType, TableType,
+    Symbols.emplace_back(std::move(Info), GlobalType, TableType,
                          Signature);
     LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n");
   }
@@ -1390,7 +1387,6 @@ Error WasmObjectFile::parseGlobalSection(ReadContext &Ctx) {
 Error WasmObjectFile::parseExportSection(ReadContext &Ctx) {
   uint32_t Count = readVaruint32(Ctx);
   Exports.reserve(Count);
-  LinkingData.SymbolTable.reserve(Count);
   Symbols.reserve(Count);
   for (uint32_t I = 0; I < Count; I++) {
     wasm::WasmExport Ex;
@@ -1455,8 +1451,7 @@ Error WasmObjectFile::parseExportSection(ReadContext &Ctx) {
     }
     Exports.push_back(Ex);
     if (Ex.Kind != wasm::WASM_EXTERNAL_MEMORY) {
-      LinkingData.SymbolTable.emplace_back(Info);
-      Symbols.emplace_back(LinkingData.SymbolTable.back(), GlobalType,
+      Symbols.emplace_back(std::move(Info), GlobalType,
                            TableType, Signature);
       LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n");
     }
diff --git a/llvm/tools/obj2yaml/wasm2yaml.cpp b/llvm/tools/obj2yaml/wasm2yaml.cpp
index c15d64cef8d65..b2679fb28857f 100644
--- a/llvm/tools/obj2yaml/wasm2yaml.cpp
+++ b/llvm/tools/obj2yaml/wasm2yaml.cpp
@@ -124,7 +124,8 @@ WasmDumper::dumpCustomSection(const WasmSection &WasmSec) {
     }
 
     uint32_t SymbolIndex = 0;
-    for (const wasm::WasmSymbolInfo &Symbol : Obj.linkingData().SymbolTable) {
+    for (const auto &Sym : Obj.symbols()) {
+      const wasm::WasmSymbolInfo& Symbol = Obj.getWasmSymbol(Sym).Info;
       WasmYAML::SymbolInfo Info;
       Info.Index = SymbolIndex++;
       Info.Kind = static_cast<uint32_t>(Symbol.Kind);

>From d6e17c91fa9d9199a44370416a8fd642edb9caaf Mon Sep 17 00:00:00 2001
From: Derek Schuff <dschuff at chromium.org>
Date: Thu, 1 Feb 2024 14:28:01 -0800
Subject: [PATCH 2/5] add comments

---
 llvm/include/llvm/BinaryFormat/Wasm.h | 5 +++++
 llvm/include/llvm/Object/Wasm.h       | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/llvm/include/llvm/BinaryFormat/Wasm.h b/llvm/include/llvm/BinaryFormat/Wasm.h
index 9c1cf68d939a9..aec6ea0b75779 100644
--- a/llvm/include/llvm/BinaryFormat/Wasm.h
+++ b/llvm/include/llvm/BinaryFormat/Wasm.h
@@ -467,10 +467,15 @@ struct WasmDebugName {
   StringRef Name;
 };
 
+// Info from the linking metadata section of a wasm object file.
 struct WasmLinkingData {
   uint32_t Version;
   std::vector<WasmInitFunc> InitFunctions;
   std::vector<StringRef> Comdats;
+  // The linking section also contains a symbol table. This info (represented
+  // in a WasmSymbolInfo struct) is stored inside the WasmSymbol object instead
+  // of in this structure; this allows vectors of WasmSymbols and
+  // WasmLinkingDatas to be reallocated.
 };
 
 struct WasmSignature {
diff --git a/llvm/include/llvm/Object/Wasm.h b/llvm/include/llvm/Object/Wasm.h
index 1a222e58ef906..95e42a22f3b78 100644
--- a/llvm/include/llvm/Object/Wasm.h
+++ b/llvm/include/llvm/Object/Wasm.h
@@ -43,6 +43,8 @@ class WasmSymbol {
     assert(!Signature || Signature->Kind != wasm::WasmSignature::Placeholder);
   }
 
+  // Symbol info as represented in the symbol's 'syminfo' entry of an object
+  // file's symbol table.
   const wasm::WasmSymbolInfo Info;
   const wasm::WasmGlobalType *GlobalType;
   const wasm::WasmTableType *TableType;

>From a45ec69bbb7c6835891f812ad11c5d0b29216e6e Mon Sep 17 00:00:00 2001
From: Derek Schuff <dschuff at chromium.org>
Date: Thu, 1 Feb 2024 15:07:54 -0800
Subject: [PATCH 3/5] clean up

---
 lld/wasm/InputFiles.cpp            | 2 +-
 llvm/include/llvm/Object/Wasm.h    | 4 ++--
 llvm/lib/Object/WasmObjectFile.cpp | 4 ++--
 llvm/tools/obj2yaml/wasm2yaml.cpp  | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp
index 394ca98d1265a..60ee60d9a3aa9 100644
--- a/lld/wasm/InputFiles.cpp
+++ b/lld/wasm/InputFiles.cpp
@@ -335,7 +335,7 @@ void ObjFile::addLegacyIndirectFunctionTableIfNeeded(
   const WasmGlobalType *globalType = nullptr;
   const WasmSignature *signature = nullptr;
   auto *wasmSym =
-      make<WasmSymbol>(std::move(info), globalType, &tableImport->Table, signature);
+      make<WasmSymbol>(info, globalType, &tableImport->Table, signature);
   Symbol *sym = createUndefined(*wasmSym, false);
   // We're only sure it's a TableSymbol if the createUndefined succeeded.
   if (errorCount())
diff --git a/llvm/include/llvm/Object/Wasm.h b/llvm/include/llvm/Object/Wasm.h
index 95e42a22f3b78..13d9a17e24c3d 100644
--- a/llvm/include/llvm/Object/Wasm.h
+++ b/llvm/include/llvm/Object/Wasm.h
@@ -34,7 +34,7 @@ namespace object {
 
 class WasmSymbol {
 public:
-  WasmSymbol(const wasm::WasmSymbolInfo &&Info,
+  WasmSymbol(const wasm::WasmSymbolInfo &Info,
              const wasm::WasmGlobalType *GlobalType,
              const wasm::WasmTableType *TableType,
              const wasm::WasmSignature *Signature)
@@ -45,7 +45,7 @@ class WasmSymbol {
 
   // Symbol info as represented in the symbol's 'syminfo' entry of an object
   // file's symbol table.
-  const wasm::WasmSymbolInfo Info;
+  wasm::WasmSymbolInfo Info;
   const wasm::WasmGlobalType *GlobalType;
   const wasm::WasmTableType *TableType;
   const wasm::WasmSignature *Signature;
diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index 636ec484e2d05..7e10bd8071aee 100644
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -842,7 +842,7 @@ Error WasmObjectFile::parseLinkingSectionSymtab(ReadContext &Ctx) {
       return make_error<GenericBinaryError>("duplicate symbol name " +
                                                 Twine(Info.Name),
                                             object_error::parse_failed);
-    Symbols.emplace_back(std::move(Info), GlobalType, TableType,
+    Symbols.emplace_back(Info, GlobalType, TableType,
                          Signature);
     LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n");
   }
@@ -1451,7 +1451,7 @@ Error WasmObjectFile::parseExportSection(ReadContext &Ctx) {
     }
     Exports.push_back(Ex);
     if (Ex.Kind != wasm::WASM_EXTERNAL_MEMORY) {
-      Symbols.emplace_back(std::move(Info), GlobalType,
+      Symbols.emplace_back(Info, GlobalType,
                            TableType, Signature);
       LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n");
     }
diff --git a/llvm/tools/obj2yaml/wasm2yaml.cpp b/llvm/tools/obj2yaml/wasm2yaml.cpp
index b2679fb28857f..397b1fd0ab81f 100644
--- a/llvm/tools/obj2yaml/wasm2yaml.cpp
+++ b/llvm/tools/obj2yaml/wasm2yaml.cpp
@@ -124,7 +124,7 @@ WasmDumper::dumpCustomSection(const WasmSection &WasmSec) {
     }
 
     uint32_t SymbolIndex = 0;
-    for (const auto &Sym : Obj.symbols()) {
+    for (const object::SymbolRef &Sym : Obj.symbols()) {
       const wasm::WasmSymbolInfo& Symbol = Obj.getWasmSymbol(Sym).Info;
       WasmYAML::SymbolInfo Info;
       Info.Index = SymbolIndex++;

>From b2d746946e4b1d053d183da0242e5fa56b7c1503 Mon Sep 17 00:00:00 2001
From: Derek Schuff <dschuff at chromium.org>
Date: Thu, 1 Feb 2024 15:12:21 -0800
Subject: [PATCH 4/5] clang-format

---
 llvm/lib/Object/WasmObjectFile.cpp | 6 ++----
 llvm/tools/obj2yaml/wasm2yaml.cpp  | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/Object/WasmObjectFile.cpp b/llvm/lib/Object/WasmObjectFile.cpp
index 7e10bd8071aee..4778562779bb7 100644
--- a/llvm/lib/Object/WasmObjectFile.cpp
+++ b/llvm/lib/Object/WasmObjectFile.cpp
@@ -842,8 +842,7 @@ Error WasmObjectFile::parseLinkingSectionSymtab(ReadContext &Ctx) {
       return make_error<GenericBinaryError>("duplicate symbol name " +
                                                 Twine(Info.Name),
                                             object_error::parse_failed);
-    Symbols.emplace_back(Info, GlobalType, TableType,
-                         Signature);
+    Symbols.emplace_back(Info, GlobalType, TableType, Signature);
     LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n");
   }
 
@@ -1451,8 +1450,7 @@ Error WasmObjectFile::parseExportSection(ReadContext &Ctx) {
     }
     Exports.push_back(Ex);
     if (Ex.Kind != wasm::WASM_EXTERNAL_MEMORY) {
-      Symbols.emplace_back(Info, GlobalType,
-                           TableType, Signature);
+      Symbols.emplace_back(Info, GlobalType, TableType, Signature);
       LLVM_DEBUG(dbgs() << "Adding symbol: " << Symbols.back() << "\n");
     }
   }
diff --git a/llvm/tools/obj2yaml/wasm2yaml.cpp b/llvm/tools/obj2yaml/wasm2yaml.cpp
index 397b1fd0ab81f..9aa7f5abe8dd4 100644
--- a/llvm/tools/obj2yaml/wasm2yaml.cpp
+++ b/llvm/tools/obj2yaml/wasm2yaml.cpp
@@ -125,7 +125,7 @@ WasmDumper::dumpCustomSection(const WasmSection &WasmSec) {
 
     uint32_t SymbolIndex = 0;
     for (const object::SymbolRef &Sym : Obj.symbols()) {
-      const wasm::WasmSymbolInfo& Symbol = Obj.getWasmSymbol(Sym).Info;
+      const wasm::WasmSymbolInfo &Symbol = Obj.getWasmSymbol(Sym).Info;
       WasmYAML::SymbolInfo Info;
       Info.Index = SymbolIndex++;
       Info.Kind = static_cast<uint32_t>(Symbol.Kind);

>From a5d53a1b100068e4f3253f775db7b3085f29ff34 Mon Sep 17 00:00:00 2001
From: Derek Schuff <dschuff at chromium.org>
Date: Fri, 2 Feb 2024 10:40:55 -0800
Subject: [PATCH 5/5] review comment

---
 lld/wasm/InputFiles.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lld/wasm/InputFiles.cpp b/lld/wasm/InputFiles.cpp
index 60ee60d9a3aa9..473208a08a812 100644
--- a/lld/wasm/InputFiles.cpp
+++ b/lld/wasm/InputFiles.cpp
@@ -15,6 +15,7 @@
 #include "lld/Common/Args.h"
 #include "lld/Common/CommonLinkerContext.h"
 #include "lld/Common/Reproduce.h"
+#include "llvm/BinaryFormat/Wasm.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/Wasm.h"
 #include "llvm/Support/Path.h"
@@ -327,8 +328,7 @@ void ObjFile::addLegacyIndirectFunctionTableIfNeeded(
   info.Kind = WASM_SYMBOL_TYPE_TABLE;
   info.ImportModule = tableImport->Module;
   info.ImportName = tableImport->Field;
-  info.Flags = WASM_SYMBOL_UNDEFINED;
-  info.Flags |= WASM_SYMBOL_NO_STRIP;
+  info.Flags = WASM_SYMBOL_UNDEFINED | WASM_SYMBOL_NO_STRIP;
   info.ElementIndex = 0;
   LLVM_DEBUG(dbgs() << "Synthesizing symbol for table import: " << info.Name
                     << "\n");



More information about the llvm-commits mailing list