[lld] r196504 - [PECOFF] Handle .lib files as if they are grouped by --{start, end}-group.

Rui Ueyama ruiu at google.com
Thu Dec 5 05:07:49 PST 2013


Author: ruiu
Date: Thu Dec  5 07:07:49 2013
New Revision: 196504

URL: http://llvm.org/viewvc/llvm-project?rev=196504&view=rev
Log:
[PECOFF] Handle .lib files as if they are grouped by --{start,end}-group.

Currently we do not de-duplicate library files specified by /defaultlib option.
As a result, the same files are added multiple times to the input graph. In
particular, some popular files, such as kernel32.lib or oldnames.lib, are added
more than 10 times during linking of LLD. That makes the linker slower, as it
needs to parse the same file again and again.

This patch solves the issue by de-duplicating. The same file will be added only
once to the input graph. This patch improved the LLD linking time from 10.5
seconds to 7.7 seconds on my 4-core Core i7 Macbook Pro.

Modified:
    lld/trunk/include/lld/Driver/WinLinkInputGraph.h
    lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h
    lld/trunk/lib/Driver/WinLinkDriver.cpp
    lld/trunk/unittests/DriverTests/DriverTest.h
    lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp

Modified: lld/trunk/include/lld/Driver/WinLinkInputGraph.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/Driver/WinLinkInputGraph.h?rev=196504&r1=196503&r2=196504&view=diff
==============================================================================
--- lld/trunk/include/lld/Driver/WinLinkInputGraph.h (original)
+++ lld/trunk/include/lld/Driver/WinLinkInputGraph.h Thu Dec  5 07:07:49 2013
@@ -61,6 +61,27 @@ public:
   virtual ErrorOr<StringRef> getPath(const LinkingContext &ctx) const;
 };
 
+/// \brief Represents a ELF control node
+class PECOFFGroup : public Group {
+public:
+  PECOFFGroup() : Group(0) {}
+
+  static inline bool classof(const InputElement *a) {
+    return a->kind() == InputElement::Kind::Control;
+  }
+
+  virtual bool validate() { return true; }
+  virtual bool dump(raw_ostream &) { return true; }
+
+  /// \brief Parse the group members.
+  error_code parse(const LinkingContext &ctx, raw_ostream &diagnostics) {
+    for (auto &elem : _elements)
+      if (error_code ec = elem->parse(ctx, diagnostics))
+        return ec;
+    return error_code::success();
+  }
+};
+
 } // namespace lld
 
 #endif

Modified: lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h?rev=196504&r1=196503&r2=196504&view=diff
==============================================================================
--- lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h (original)
+++ lld/trunk/include/lld/ReaderWriter/PECOFFLinkingContext.h Thu Dec  5 07:07:49 2013
@@ -30,6 +30,7 @@ using llvm::COFF::WindowsSubsystem;
 static const uint8_t DEFAULT_DOS_STUB[128] = {'M', 'Z'};
 
 namespace lld {
+class Group;
 
 class PECOFFLinkingContext : public LinkingContext {
 public:
@@ -235,6 +236,9 @@ public:
     return false;
   }
 
+  void setLibraryGroup(Group *group) { _libraryGroup = group; }
+  Group *getLibraryGroup() const { return _libraryGroup; }
+
 protected:
   /// Method to create a internal file for the entry symbol
   virtual std::unique_ptr<File> createEntrySymbolFile() const;
@@ -299,6 +303,9 @@ private:
   // a small DOS program that prints out a message "This program requires
   // Microsoft Windows." This feature was somewhat useful before Windows 95.
   ArrayRef<uint8_t> _dosStub;
+
+  // The PECOFFGroup that contains all the .lib files.
+  Group *_libraryGroup;
 };
 
 } // end namespace lld

Modified: lld/trunk/lib/Driver/WinLinkDriver.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/WinLinkDriver.cpp?rev=196504&r1=196503&r2=196504&view=diff
==============================================================================
--- lld/trunk/lib/Driver/WinLinkDriver.cpp (original)
+++ lld/trunk/lib/Driver/WinLinkDriver.cpp Thu Dec  5 07:07:49 2013
@@ -564,6 +564,19 @@ parseArgs(int argc, const char *argv[],
   return parsedArgs;
 }
 
+// Returns true if the given file node has already been added to the input
+// graph.
+bool hasLibrary(const PECOFFLinkingContext &ctx, FileNode *fileNode) {
+  ErrorOr<StringRef> path = fileNode->getPath(ctx);
+  if (!path)
+    return false;
+  for (std::unique_ptr<InputElement> &p : ctx.getLibraryGroup()->elements())
+    if (auto *f = dyn_cast<FileNode>(p.get()))
+      if (*path == *f->getPath(ctx))
+        return true;
+  return false;
+}
+
 } // namespace
 
 //
@@ -597,7 +610,8 @@ WinLinkDriver::parse(int argc, const cha
     return false;
 
   // The list of input files.
-  std::vector<std::unique_ptr<InputElement> > inputElements;
+  std::vector<std::unique_ptr<FileNode> > files;
+  std::vector<std::unique_ptr<FileNode> > libraries;
 
   // Handle /help
   if (parsedArgs->getLastArg(OPT_help)) {
@@ -899,8 +913,7 @@ WinLinkDriver::parse(int argc, const cha
   };
   std::stable_sort(inputFiles.begin(), inputFiles.end(), compfn);
   for (StringRef path : inputFiles)
-    inputElements.push_back(std::unique_ptr<InputElement>(
-        new PECOFFFileNode(ctx, path)));
+    files.push_back(std::unique_ptr<FileNode>(new PECOFFFileNode(ctx, path)));
 
   // Use the default entry name if /entry option is not given.
   if (ctx.entrySymbolName().empty() && !parsedArgs->getLastArg(OPT_noentry))
@@ -933,9 +946,9 @@ WinLinkDriver::parse(int argc, const cha
   // but useful for us to test lld on Unix.
   if (llvm::opt::Arg *dashdash = parsedArgs->getLastArg(OPT_DASH_DASH)) {
     for (const StringRef value : dashdash->getValues()) {
-      std::unique_ptr<InputElement> elem(
+      std::unique_ptr<FileNode> elem(
           new PECOFFFileNode(ctx, ctx.allocate(value)));
-      inputElements.push_back(std::move(elem));
+      files.push_back(std::move(elem));
     }
   }
 
@@ -944,10 +957,10 @@ WinLinkDriver::parse(int argc, const cha
   if (!ctx.getNoDefaultLibAll())
     for (const StringRef path : defaultLibs)
       if (!ctx.hasNoDefaultLib(path))
-        inputElements.push_back(std::unique_ptr<InputElement>(
-            new PECOFFLibraryNode(ctx, path)));
+        libraries.push_back(std::unique_ptr<FileNode>(
+                              new PECOFFLibraryNode(ctx, ctx.allocate(path.lower()))));
 
-  if (inputElements.empty() && !isReadingDirectiveSection) {
+  if (files.empty() && !isReadingDirectiveSection) {
     diagnostics << "No input files\n";
     return false;
   }
@@ -956,7 +969,7 @@ WinLinkDriver::parse(int argc, const cha
   // constructed by replacing an extension of the first input file
   // with ".exe".
   if (ctx.outputPath().empty()) {
-    StringRef path = *dyn_cast<FileNode>(&*inputElements[0])->getPath(ctx);
+    StringRef path = *dyn_cast<FileNode>(&*files[0])->getPath(ctx);
     ctx.setOutputPath(replaceExtension(ctx, path, ".exe"));
   }
 
@@ -968,19 +981,32 @@ WinLinkDriver::parse(int argc, const cha
     ctx.setManifestOutputPath(ctx.allocate(path));
   }
 
-  // If the core linker already started, we need to explicitly call parse() for
-  // each input element, because the pass to parse input files in Driver::link
-  // has already done.
-  if (isReadingDirectiveSection)
-    for (auto &e : inputElements)
-      if (e->parse(ctx, diagnostics))
-        return false;
-
   // Add the input files to the input graph.
   if (!ctx.hasInputGraph())
     ctx.setInputGraph(std::unique_ptr<InputGraph>(new InputGraph()));
-  for (auto &e : inputElements)
-    ctx.inputGraph().addInputElement(std::move(e));
+  for (auto &file : files) {
+    if (isReadingDirectiveSection)
+      if (file->parse(ctx, diagnostics))
+        return false;
+    ctx.inputGraph().addInputElement(std::move(file));
+  }
+
+  // Add the library group to the input graph.
+  if (!isReadingDirectiveSection) {
+    auto group = std::unique_ptr<Group>(new PECOFFGroup());
+    ctx.setLibraryGroup(group.get());
+    ctx.inputGraph().addInputElement(std::move(group));
+  }
+
+  // Add the library files to the library group.
+  for (std::unique_ptr<FileNode> &lib : libraries) {
+    if (!hasLibrary(ctx, lib.get())) {
+      if (isReadingDirectiveSection)
+        if (lib->parse(ctx, diagnostics))
+          return false;
+      ctx.getLibraryGroup()->processInputElement(std::move(lib));
+    }
+  }
 
   // Validate the combination of options used.
   return ctx.validate(diagnostics);

Modified: lld/trunk/unittests/DriverTests/DriverTest.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/unittests/DriverTests/DriverTest.h?rev=196504&r1=196503&r2=196504&view=diff
==============================================================================
--- lld/trunk/unittests/DriverTests/DriverTest.h (original)
+++ lld/trunk/unittests/DriverTests/DriverTest.h Thu Dec  5 07:07:49 2013
@@ -32,13 +32,24 @@ protected:
   int inputFileCount() { return linkingContext()->inputGraph().size(); }
 
   // Convenience method for getting i'th input files name.
-  std::string inputFile(unsigned index) {
+  std::string inputFile(int index) {
     const InputElement &inputElement = linkingContext()->inputGraph()[index];
     if (inputElement.kind() == InputElement::Kind::File)
       return *dyn_cast<FileNode>(&inputElement)->getPath(*linkingContext());
     llvm_unreachable("not handling other types of input files");
   }
 
+  // Convenience method for getting i'th input files name.
+  std::string inputFile(int index1, int index2) {
+    Group *group = dyn_cast<Group>(&linkingContext()->inputGraph()[index1]);
+    if (!group)
+      llvm_unreachable("not handling other types of input files");
+    FileNode *file = dyn_cast<FileNode>(group->elements()[index2].get());
+    if (!file)
+      llvm_unreachable("not handling other types of input files");
+    return *file->getPath(*linkingContext());
+  }
+
   // For unit tests to call driver with various command lines.
   bool parse(const char *args, ...) {
     // Construct command line options from varargs.

Modified: lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp?rev=196504&r1=196503&r2=196504&view=diff
==============================================================================
--- lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp (original)
+++ lld/trunk/unittests/DriverTests/WinLinkDriverTest.cpp Thu Dec  5 07:07:49 2013
@@ -38,7 +38,7 @@ TEST_F(WinLinkParserTest, Basic) {
   EXPECT_EQ(llvm::COFF::IMAGE_FILE_MACHINE_I386, _context.getMachineType());
   EXPECT_EQ("a.exe", _context.outputPath());
   EXPECT_EQ("_start", _context.entrySymbolName());
-  EXPECT_EQ(3, inputFileCount());
+  EXPECT_EQ(4, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
   EXPECT_EQ("b.obj", inputFile(1));
   EXPECT_EQ("c.obj", inputFile(2));
@@ -77,7 +77,7 @@ TEST_F(WinLinkParserTest, StartsWithHyph
       parse("link.exe", "-subsystem:console", "-out:a.exe", "a.obj", nullptr));
   EXPECT_EQ(llvm::COFF::IMAGE_SUBSYSTEM_WINDOWS_CUI, _context.getSubsystem());
   EXPECT_EQ("a.exe", _context.outputPath());
-  EXPECT_EQ(1, inputFileCount());
+  EXPECT_EQ(2, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
 }
 
@@ -86,7 +86,7 @@ TEST_F(WinLinkParserTest, UppercaseOptio
       parse("link.exe", "/SUBSYSTEM:CONSOLE", "/OUT:a.exe", "a.obj", nullptr));
   EXPECT_EQ(llvm::COFF::IMAGE_SUBSYSTEM_WINDOWS_CUI, _context.getSubsystem());
   EXPECT_EQ("a.exe", _context.outputPath());
-  EXPECT_EQ(1, inputFileCount());
+  EXPECT_EQ(2, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
 }
 
@@ -109,7 +109,7 @@ TEST_F(WinLinkParserTest, NoInputFiles)
 TEST_F(WinLinkParserTest, NoFileExtension) {
   EXPECT_TRUE(parse("link.exe", "foo", "bar", nullptr));
   EXPECT_EQ("foo.exe", _context.outputPath());
-  EXPECT_EQ(2, inputFileCount());
+  EXPECT_EQ(3, inputFileCount());
   EXPECT_EQ("foo.obj", inputFile(0));
   EXPECT_EQ("bar.obj", inputFile(1));
 }
@@ -117,7 +117,7 @@ TEST_F(WinLinkParserTest, NoFileExtensio
 TEST_F(WinLinkParserTest, NonStandardFileExtension) {
   EXPECT_TRUE(parse("link.exe", "foo.o", nullptr));
   EXPECT_EQ("foo.exe", _context.outputPath());
-  EXPECT_EQ(1, inputFileCount());
+  EXPECT_EQ(2, inputFileCount());
   EXPECT_EQ("foo.o", inputFile(0));
 }
 
@@ -137,7 +137,7 @@ TEST_F(WinLinkParserTest, Libpath) {
 TEST_F(WinLinkParserTest, InputOrder) {
   EXPECT_TRUE(parse("link.exe", "b.lib", "b.obj", "c.obj", "a.lib", "a.obj",
                     nullptr));
-  EXPECT_EQ(5, inputFileCount());
+  EXPECT_EQ(6, inputFileCount());
   EXPECT_EQ("b.obj", inputFile(0));
   EXPECT_EQ("c.obj", inputFile(1));
   EXPECT_EQ("a.obj", inputFile(2));
@@ -319,19 +319,18 @@ TEST_F(WinLinkParserTest, SectionMultipl
 TEST_F(WinLinkParserTest, DefaultLib) {
   EXPECT_TRUE(parse("link.exe", "/defaultlib:user32.lib",
                     "/defaultlib:kernel32", "a.obj", nullptr));
-  EXPECT_EQ(3, inputFileCount());
+  EXPECT_EQ(2, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
-  EXPECT_EQ("user32.lib", inputFile(1));
-  EXPECT_EQ("kernel32.lib", inputFile(2));
+  EXPECT_EQ("user32.lib", inputFile(1, 0));
+  EXPECT_EQ("kernel32.lib", inputFile(1, 1));
 }
 
 TEST_F(WinLinkParserTest, DefaultLibDuplicates) {
   EXPECT_TRUE(parse("link.exe", "/defaultlib:user32.lib",
                     "/defaultlib:user32.lib", "a.obj", nullptr));
-  EXPECT_EQ(3, inputFileCount());
+  EXPECT_EQ(2, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
-  EXPECT_EQ("user32.lib", inputFile(1));
-  EXPECT_EQ("user32.lib", inputFile(2));
+  EXPECT_EQ("user32.lib", inputFile(1, 0));
 }
 
 TEST_F(WinLinkParserTest, NoDefaultLib) {
@@ -340,13 +339,13 @@ TEST_F(WinLinkParserTest, NoDefaultLib)
                     nullptr));
   EXPECT_EQ(2, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
-  EXPECT_EQ("kernel32.lib", inputFile(1));
+  EXPECT_EQ("kernel32.lib", inputFile(1, 0));
 }
 
 TEST_F(WinLinkParserTest, NoDefaultLibAll) {
   EXPECT_TRUE(parse("link.exe", "/defaultlib:user32.lib",
                     "/defaultlib:kernel32", "/nodefaultlib", "a.obj", nullptr));
-  EXPECT_EQ(1, inputFileCount());
+  EXPECT_EQ(2, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
 }
 
@@ -356,7 +355,7 @@ TEST_F(WinLinkParserTest, DisallowLib) {
                     nullptr));
   EXPECT_EQ(2, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
-  EXPECT_EQ("kernel32.lib", inputFile(1));
+  EXPECT_EQ("kernel32.lib", inputFile(1, 0));
 }
 
 //
@@ -567,7 +566,7 @@ TEST_F(WinLinkParserTest, Ignore) {
                     "/ignoreidl", "/implib:foo", "/safeseh", "/safeseh:no",
                     "/functionpadmin", "a.obj", nullptr));
   EXPECT_EQ("", errorMessage());
-  EXPECT_EQ(1, inputFileCount());
+  EXPECT_EQ(2, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
 }
 
@@ -580,7 +579,7 @@ TEST_F(WinLinkParserTest, DashDash) {
                     "--", "b.obj", "-c.obj", nullptr));
   EXPECT_EQ(llvm::COFF::IMAGE_SUBSYSTEM_WINDOWS_CUI, _context.getSubsystem());
   EXPECT_EQ("a.exe", _context.outputPath());
-  EXPECT_EQ(3, inputFileCount());
+  EXPECT_EQ(4, inputFileCount());
   EXPECT_EQ("a.obj", inputFile(0));
   EXPECT_EQ("b.obj", inputFile(1));
   EXPECT_EQ("-c.obj", inputFile(2));





More information about the llvm-commits mailing list