[Clang][Bundler] Reduce fat object size

[sdmitriev]

Summary

Fat object size has significantly increased after D65819 which changed bundler tool to add host object as a normal bundle to the fat output which almost doubled its size. That patch was fixing the following issues

1. Problems associated with the partial linking - global constructors were not called for partially linking objects which clearly resulted in incorrect behavior.
2. Eliminating "junk" target object sections from the linked binary on the host side.

The first problem is no longer relevant because we do not use partial linking for creating fat objects anymore. Target objects sections are now inserted into the resulting fat object with a help of llvm-objcopy tool.

The second issue, "junk" sections in the linked host binary, has been fixed in D73408 by adding "exclude" flag to the fat object's sections which contain target objects. This flag tells linker to drop section from the inputs when linking executable or shared library, therefore these sections will not be propagated in the linked binary.

Since both problems have been solved, we can revert D65819 changes to reduce fat object size and this patch essentially is doing that.

[ABataev]

clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

528–553

We usually don't do this. New types must be declared outside in anonymous namespaces. Also, better to use part-of inheritance rather than is-a here.

[sdmitriev]

Addressed review comments.

[ABataev]

clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

371

If you have private members, it should be a class. Also, seems to me it is a RAII class. Better to name it something like TempFileHandlerRAII or something like this.

378

I see that Contents is either None or just one byte 0. Maybe, better to use bool flag here instead of Contents?

[sdmitriev]

Addressed review comments.

[sdmitriev]

clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

378

Well, maybe we will need some other contents besides one zero byte in future, so I think having optional contents is probably better.

[sdmitriev]

@ABataev, do you have any additional comments?

[ABataev]

LG

clang/test/Driver/clang-offload-bundler.c

 // section (we are using a PowerPC little-endian host which uses ELF). We
 // have an already bundled file to check the unbundle and do a dry run on the
 // bundling as it cannot be tested in all host platforms that will run these
 // tests.
 //
 
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.o,%t.tgt1,%t.tgt2 -outputs=%t.bundle3.o -### 2>&1 \
 // RUN: | FileCheck %s -DHOST=%itanium_abi_triple -DINOBJ1=%t.o -DINOBJ2=%t.tgt1 -DINOBJ3=%t.tgt2 -DOUTOBJ=%t.bundle3.o --check-prefix CK-OBJ-CMD
-// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=[[INOBJ1]]" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" "[[INOBJ1]]" "[[TEMPOBJ:.*]]"
+// CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]={{.*}}" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=[[INOBJ2]]" "--add-section=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=[[INOBJ3]]" "[[INOBJ1]]" "[[TEMPOBJ:.*]]"
 // CK-OBJ-CMD: llvm-objcopy{{(.exe)?}}" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__host-[[HOST]]=readonly,exclude" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-powerpc64le-ibm-linux-gnu=readonly,exclude" "--set-section-flags=__CLANG_OFFLOAD_BUNDLE__openmp-x86_64-pc-linux-gnu=readonly,exclude" "[[TEMPOBJ]]" "[[OUTOBJ]]"
 
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -inputs=%t.o,%t.tgt1,%t.tgt2 -outputs=%t.bundle3.o
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.o,%t.res.tgt1,%t.res.tgt2 -inputs=%t.bundle3.o -unbundle
-// RUN: diff %t.o %t.res.o
+// RUN: diff %t.bundle3.o %t.res.o
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
 // RUN: clang-offload-bundler -type=o -targets=openmp-powerpc64le-ibm-linux-gnu,host-%itanium_abi_triple,openmp-x86_64-pc-linux-gnu -outputs=%t.res.tgt1,%t.res.o,%t.res.tgt2 -inputs=%t.bundle3.o -unbundle
-// RUN: diff %t.o %t.res.o
+// RUN: diff %t.bundle3.o %t.res.o
 // RUN: diff %t.tgt1 %t.res.tgt1
 // RUN: diff %t.tgt2 %t.res.tgt2
 // RUN: clang-offload-bundler -type=o -targets=openmp-powerpc64le-ibm-linux-gnu -outputs=%t.res.tgt1 -inputs=%t.bundle3.o -unbundle
 // RUN: diff %t.tgt1 %t.res.tgt1
 
 // Check if we can unbundle a file with no magic strings.
 // RUN: clang-offload-bundler -type=o -targets=host-%itanium_abi_triple,openmp-powerpc64le-ibm-linux-gnu,openmp-x86_64-pc-linux-gnu -outputs=%t.res.o,%t.res.tgt1,%t.res.tgt2 -inputs=%t.o -unbundle
 // RUN: diff %t.o %t.res.o

clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

 #include "llvm/Object/Binary.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
public:
   }
 
   Error WriteBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
     OS.write(Input.getBufferStart(), Input.getBufferSize());
     return Error::success();
   }
 };
 
+namespace {
+
+// This class implements a list of temporary files that are removed upon
+// object destruction.
+class TempFileHandlerRAII {
+public:
+  ~TempFileHandlerRAII() {
+    for (const auto &File : Files)
+      sys::fs::remove(File);
+  }
+
+  // Creates temporary file with given contents.
+  Expected<StringRef> Create(Optional<ArrayRef<char>> Contents) {
+    SmallString<128u> File;
+    if (std::error_code EC =
+            sys::fs::createTemporaryFile("clang-offload-bundler", "tmp", File))
+      return createFileError(File, EC);
+    Files.push_back(File);
+
+    if (Contents) {
+      std::error_code EC;
+      raw_fd_ostream OS(File, EC);
+      if (EC)
+        return createFileError(File, EC);
+      OS.write(Contents->data(), Contents->size());
+    }
+    return Files.back();
+  }
+
+private:
+  SmallVector<SmallString<128u>, 4u> Files;
+};
+
+} // end anonymous namespace
+
 /// Handler for object files. The bundles are organized by sections with a
 /// designated name.
 ///
 /// To unbundle, we just copy the contents of the designated section.
 class ObjectFileHandler final : public FileHandler {
 
   /// The object file we are currently dealing with.
   std::unique_ptr<ObjectFile> Obj;
while (NextSection != Obj->section_end()) {
         return **TripleOrErr;
     }
     return None;
   }
 
   Error ReadBundleEnd(MemoryBuffer &Input) final { return Error::success(); }
 
   Error ReadBundle(raw_fd_ostream &OS, MemoryBuffer &Input) final {
-    Expected<StringRef> Content = CurrentSection->getContents();
-    if (!Content)
-      return Content.takeError();
+    Expected<StringRef> ContentOrErr = CurrentSection->getContents();
+    if (!ContentOrErr)
+      return ContentOrErr.takeError();
+    StringRef Content = *ContentOrErr;
+
+    // Copy fat object contents to the output when extracting host bundle.
+    if (Content.size() == 1u && Content.front() == 0)
+      Content = StringRef(Input.getBufferStart(), Input.getBufferSize());
 
-    OS.write(Content->data(), Content->size());
+    OS.write(Content.data(), Content.size());
     return Error::success();
   }
 
   Error WriteHeader(raw_fd_ostream &OS,
                     ArrayRef<std::unique_ptr<MemoryBuffer>> Inputs) final {
     assert(HostInputIndex != ~0u && "Host input index not defined.");
 
     // Record number of inputs.
Error WriteBundleEnd(raw_fd_ostream &OS, StringRef TargetTriple) final {
     if (!Objcopy)
       return createStringError(Objcopy.getError(),
                                "unable to find 'llvm-objcopy' in path");
 
     // We write to the output file directly. So, we close it and use the name
     // to pass down to llvm-objcopy.
     OS.close();
 
+    // Temporary files that need to be removed.
+    TempFileHandlerRAII TempFiles;
+
     // Create an intermediate temporary file to save object after the first
     // llvm-objcopy run.
-    SmallString<128u> IntermediateObj;
-    if (std::error_code EC = sys::fs::createTemporaryFile(
-            "clang-offload-bundler", "tmp", IntermediateObj))
-      return createFileError(IntermediateObj, EC);
-    FileRemover IntermediateObjRemover(IntermediateObj);
+    Expected<SmallString<128u>> IntermediateObjOrErr = TempFiles.Create(None);
+    if (!IntermediateObjOrErr)
+      return IntermediateObjOrErr.takeError();
+    const SmallString<128u> &IntermediateObj = *IntermediateObjOrErr;
 
     // Compose llvm-objcopy command line for add target objects' sections.
     BumpPtrAllocator Alloc;
     StringSaver SS{Alloc};
     SmallVector<StringRef, 8u> ObjcopyArgs{"llvm-objcopy"};
-    for (unsigned I = 0; I < NumberOfInputs; ++I)
+    for (unsigned I = 0; I < NumberOfInputs; ++I) {
+      StringRef InputFile = InputFileNames[I];
+      if (I == HostInputIndex) {
+        // Special handling for the host bundle. We do not need to add a
+        // standard bundle for the host object since we are going to use fat
+        // object as a host object. Therefore use dummy contents (one zero byte)
+        // when creating section for the host bundle.
+        Expected<StringRef> TempFileOrErr = TempFiles.Create(ArrayRef<char>(0));
+        if (!TempFileOrErr)
+          return TempFileOrErr.takeError();
+        InputFile = *TempFileOrErr;
+      }
+
       ObjcopyArgs.push_back(SS.save(Twine("--add-section=") +
                                     OFFLOAD_BUNDLER_MAGIC_STR + TargetNames[I] +
-                                    "=" + InputFileNames[I]));
+                                    "=" + InputFile));
+    }
     ObjcopyArgs.push_back(InputFileNames[HostInputIndex]);
     ObjcopyArgs.push_back(IntermediateObj);
 
     if (Error Err = executeObjcopy(*Objcopy, ObjcopyArgs))
       return Err;
 
     // And run llvm-objcopy for the second time to update section flags.
     ObjcopyArgs.resize(1);