[llvm] r320601 - [dsymutil] Re-enable threading

Jonas Devlieghere via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 13 10:03:04 PST 2017


Author: jdevlieghere
Date: Wed Dec 13 10:03:04 2017
New Revision: 320601

URL: http://llvm.org/viewvc/llvm-project?rev=320601&view=rev
Log:
[dsymutil] Re-enable threading

Threading was disabled in r317263 because it broke a test in combination
with `-DLLVM_ENABLE_THREADS=OFF`. This was because a ThreadPool warning
was piped to llvm-dwarfdump which was expecting to read an object from
stdin.

This patch re-enables threading and fixes the offending test.
Unfortunately this required more than just moving the ThreadPool out of
the for loop because of the TempFile refactoring that took place in the
meantime.

Differential revision: https://reviews.llvm.org/D41180

Added:
    llvm/trunk/test/tools/dsymutil/ARM/fat-threading.test
Modified:
    llvm/trunk/test/tools/dsymutil/X86/alias.test
    llvm/trunk/tools/dsymutil/dsymutil.cpp

Added: llvm/trunk/test/tools/dsymutil/ARM/fat-threading.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/ARM/fat-threading.test?rev=320601&view=auto
==============================================================================
--- llvm/trunk/test/tools/dsymutil/ARM/fat-threading.test (added)
+++ llvm/trunk/test/tools/dsymutil/ARM/fat-threading.test Wed Dec 13 10:03:04 2017
@@ -0,0 +1,15 @@
+# REQUIRES: object-emission
+# By default, dsymutil spawns one thread per architecture and this test just
+# ensures that things don't break when processing multiple archs.
+# RUN: llvm-dsymutil -no-output %p/../Inputs/fat-test.arm.dylib -o %t.brief.dSYM 2>&1
+# RUN: llvm-dsymutil -verify %p/../Inputs/fat-test.arm.dylib -o %t.verify.dSYM 2>&1
+
+# CHECK: warning: no debug symbols in executable (-arch armv7)
+# CHECK: warning: no debug symbols in executable (-arch armv7s)
+# CHECK: warning: no debug symbols in executable (-arch arm64)
+
+# CHECK: Running lipo
+# CHECK-NEXT: lipo -create
+# CHECK-SAME: -segalign armv7
+# CHECK-SAME: -segalign armv7s
+# CHECK-SAME: -segalign arm64

Modified: llvm/trunk/test/tools/dsymutil/X86/alias.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/dsymutil/X86/alias.test?rev=320601&r1=320600&r2=320601&view=diff
==============================================================================
--- llvm/trunk/test/tools/dsymutil/X86/alias.test (original)
+++ llvm/trunk/test/tools/dsymutil/X86/alias.test Wed Dec 13 10:03:04 2017
@@ -1,5 +1,5 @@
 # RUN: llvm-dsymutil -f -oso-prepend-path=%p/../Inputs/alias \
-# RUN: %p/../Inputs/alias/foobar -o - 2>&1 | llvm-dwarfdump - | FileCheck %s
+# RUN: %p/../Inputs/alias/foobar -o - | llvm-dwarfdump - 2>&1 | FileCheck %s
 # CHECK-NOT: could not find object file symbol for symbol
 # CHECK: DW_AT_name ("foo.c")
 # CHECK: DW_AT_name ("bar.c")

Modified: llvm/trunk/tools/dsymutil/dsymutil.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/dsymutil.cpp?rev=320601&r1=320600&r2=320601&view=diff
==============================================================================
--- llvm/trunk/tools/dsymutil/dsymutil.cpp (original)
+++ llvm/trunk/tools/dsymutil/dsymutil.cpp Wed Dec 13 10:03:04 2017
@@ -101,8 +101,8 @@ static list<std::string> ArchFlags(
     desc("Link DWARF debug information only for specified CPU architecture\n"
          "types. This option can be specified multiple times, once for each\n"
          "desired architecture. All CPU architectures will be linked by\n"
-         "default."), value_desc("arch"),
-    ZeroOrMore, cat(DsymCategory));
+         "default."),
+    value_desc("arch"), ZeroOrMore, cat(DsymCategory));
 
 static opt<bool>
     NoODR("no-odr",
@@ -213,7 +213,7 @@ static bool verify(llvm::StringRef Outpu
     DIDumpOptions DumpOpts;
     bool success = DICtx->verify(os, DumpOpts.noImplicitRecursion());
     if (!success)
-        errs() << "error: verification failed for " << Arch << '\n';
+      errs() << "error: verification failed for " << Arch << '\n';
     return success;
   }
 
@@ -355,12 +355,14 @@ int main(int argc, char **argv) {
       NumThreads = 1;
     NumThreads = std::min<unsigned>(NumThreads, DebugMapPtrsOrErr->size());
 
+    llvm::ThreadPool Threads(NumThreads);
 
     // If there is more than one link to execute, we need to generate
     // temporary files.
     bool NeedsTempFiles = !DumpDebugMap && (*DebugMapPtrsOrErr).size() != 1;
     llvm::SmallVector<MachOUtils::ArchAndFilename, 4> TempFiles;
     TempFileVector TempFileStore;
+    std::atomic_char AllOK(1);
     for (auto &Map : *DebugMapPtrsOrErr) {
       if (Verbose || DumpDebugMap)
         Map->print(llvm::outs());
@@ -373,55 +375,52 @@ int main(int argc, char **argv) {
                      << MachOUtils::getArchName(Map->getTriple().getArchName())
                      << ")\n";
 
+      // Using a std::shared_ptr rather than std::unique_ptr because move-only
+      // types don't work with std::bind in the ThreadPool implementation.
+      std::shared_ptr<raw_fd_ostream> OS;
       std::string OutputFile = getOutputFileName(InputFile);
-      std::unique_ptr<raw_fd_ostream> OS;
       if (NeedsTempFiles) {
         Expected<sys::fs::TempFile> T = createTempFile();
         if (!T) {
           errs() << toString(T.takeError());
           return 1;
         }
-        OS = llvm::make_unique<raw_fd_ostream>(T->FD, /*shouldClose*/ false);
+        OS = std::make_shared<raw_fd_ostream>(T->FD, /*shouldClose*/ false);
         OutputFile = T->TmpName;
         TempFileStore.Files.push_back(std::move(*T));
+        TempFiles.emplace_back(Map->getTriple().getArchName().str(),
+                               OutputFile);
       } else {
         std::error_code EC;
-        OS = llvm::make_unique<raw_fd_ostream>(NoOutput ? "-" : OutputFile, EC,
-                                         sys::fs::F_None);
+        OS = std::make_shared<raw_fd_ostream>(NoOutput ? "-" : OutputFile, EC,
+                                              sys::fs::F_None);
         if (EC) {
           errs() << OutputFile << ": " << EC.message();
           return 1;
         }
       }
 
-      std::atomic_char AllOK(1);
-      auto LinkLambda = [&]() {
-        AllOK.fetch_and(linkDwarf(*OS, *Map, Options));
-        OS->flush();
+      auto LinkLambda = [&,
+                         OutputFile](std::shared_ptr<raw_fd_ostream> Stream) {
+        AllOK.fetch_and(linkDwarf(*Stream, *Map, Options));
+        Stream->flush();
+        if (Verify && !NoOutput)
+          AllOK.fetch_and(verify(OutputFile, Map->getTriple().getArchName()));
       };
 
       // FIXME: The DwarfLinker can have some very deep recursion that can max
       // out the (significantly smaller) stack when using threads. We don't
       // want this limitation when we only have a single thread.
-      if (NumThreads == 1) {
-        LinkLambda();
-      } else {
-        llvm::ThreadPool Threads(NumThreads);
-        Threads.async(LinkLambda);
-        Threads.wait();
-      }
-      if (!AllOK)
-        return 1;
-
-      if (Verify && !NoOutput &&
-          !verify(OutputFile, Map->getTriple().getArchName()))
-        return 1;
-
-      if (NeedsTempFiles)
-        TempFiles.emplace_back(Map->getTriple().getArchName().str(),
-                               OutputFile);
+      if (NumThreads == 1)
+        LinkLambda(OS);
+      else
+        Threads.async(LinkLambda, OS);
     }
 
+    Threads.wait();
+
+    if (!AllOK)
+      return 1;
 
     if (NeedsTempFiles &&
         !MachOUtils::generateUniversalBinary(




More information about the llvm-commits mailing list