[clang][driver] Set LTO mode based on input files

[tbaeder]

Summary

After setting the LTO mode from the -flto option, look at the input files as well. If any of them is an object file containing LLVM bitcode,
set the LTO mode to either thin or full, depending on the input file.

This makes the following sample work:

clang test.c -c -flto
clang test.o

Which also works when using GCC. Currently this makes non-lld linkers print an error, for example when using ld.bfd:

$ clang test.o
test.o: file not recognized: file format not recognized
clang-10: error: linker command failed with exit code 1 (use -v to see invocation)

[MaskRay]

Is the motivation just to avoid -flto or -flto=lto at link time? I am afraid that the advantage probably is not large enough to justify the potentially costly object file parsing in the driver. Before this, as I understand it, object files are opaque to the driver. The driver just passes all file names to the linker. With this change, every object file will be opened and llvm::getBitcodeFileContents will be called on every object file.

Other thoughts include:

• An LTO link can mix regular and thin LTO bitcode files. I am not very familiar with the thinlto internals so I don't know whether it is appropriate to stop after we immediately see a full LTO bitcode file.
• This provides a convenience method in the most simplest link (no other LTO related option). If you specify any other LTO related option (e.g --thinlto-index-only, --thinlto-cache-dir), the save of a -flto option appears to give too small of a value with an extra parsing.
• clangDriver's dependency on LLVMBitReader is a bit odd.

In any case, it is just my opinion that this may not fit well as a driver feature. Hope Teresa can share more authoritative thoughts on this.

clang/lib/Driver/Driver.cpp

1205

On line 1133, there is a similar setLTOMode. If we are going to add the logic, probably consider unifying the logic. However, I am concerned with the cost, see my main comment

[tbaeder]

In D90457#2390519, @MaskRay wrote:

Is the motivation just to avoid -flto or -flto=lto at link time?

Essentially, yes. Lots of projects seem to depend on this behavior of GCC. I.e. the pass -flto when compiling but not when linking.

I am afraid that the advantage probably is not large enough to justify the potentially costly object file parsing in the driver. Before this, as I understand it, object files are opaque to the driver. The driver just passes all file names to the linker. With this change, every object file will be opened and llvm::getBitcodeFileContents will be called on every object file.

I thought about the potential performance penalties as well. All of this only matters if -flto is really missing of course, otherwise the driver is not going to do any more work than it did before.

Other thoughts include:

• An LTO link can mix regular and thin LTO bitcode files. I am not very familiar with the thinlto internals so I don't know whether it is appropriate to stop after we immediately see a full LTO bitcode file.
• This provides a convenience method in the most simplest link (no other LTO related option). If you specify any other LTO related option (e.g --thinlto-index-only, --thinlto-cache-dir), the save of a -flto option appears to give too small of a value with an extra parsing.

Right, the purpose of this is not to save typing a "-flto", but to make linking in the simplest cast just work.

clang/lib/Driver/Driver.cpp

1205

Setting the LTO mode from the input files requires the Inputs list, but calling the existing setLTOMode() this late causes problems.

[tbaeder]

Hey @MaskRay, have you thought about this some more? Or is it too much magic and you'd rather reject it altogether?

[MaskRay]

In D90457#2551698, @tbaeder wrote:

Hey @MaskRay, have you thought about this some more? Or is it too much magic and you'd rather reject it altogether?

Yes, I'm afraid it should be rejected. I've seen cases where people want clang -c a.bc to produce an object file instead of enabling LTO.

[tbaeder]

I don't think .bc files are affected by this patch since they aren't types::TY_Object files, but I might misremember.

clang/include/clang/Driver/Driver.h

private:
   /// Set the driver mode (cl, gcc, etc) from an option string of the form
   /// --driver-mode=<mode>.
   void setDriverModeFromOption(StringRef Opt);
 
   /// Parse the \p Args list for LTO options and record the type of LTO
   /// compilation based on which -f(no-)?lto(=.*)? option occurs last.
   void setLTOMode(const llvm::opt::ArgList &Args);
 
+  /// Parse the \p Inputs list for LTO inputs. LTO inputs are object files
+  /// that contain LLVM bitcode
+  void setLTOModeFromInputFiles(const InputList *Inputs);
+
   /// Retrieves a ToolChain for a particular \p Target triple.
   ///
   /// Will cache ToolChains for the life of the driver object, and create them
   /// on-demand.
   const ToolChain &getToolChain(const llvm::opt::ArgList &Args,
                                 const llvm::Triple &Target) const;
 
   /// @}

clang/lib/Driver/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   BinaryFormat
   Option
   ProfileData
   Support
+  BitReader
   )
 
 if(WIN32)
   # MSVCToolChain.cpp uses version.dll.
   set(system_libs version)
 endif()
 
 add_clang_library(clangDriver

clang/lib/Driver/Driver.cpp

 #include "clang/Driver/Tool.h"
 #include "clang/Driver/ToolChain.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallSet.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/Config/llvm-config.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/OptSpecifier.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ErrorHandling.h"
if (ArchName.startswith_lower("rv32"))
       Target.setArch(llvm::Triple::riscv32);
     else if (ArchName.startswith_lower("rv64"))
       Target.setArch(llvm::Triple::riscv64);
   }
 
   return Target;
 }
 
+static LTOKind
+getInputLTOMode(const std::pair<types::ID, const llvm::opt::Arg *> Input) {
+  // Must be an object file
+  if (Input.first != types::TY_Object)
+    return LTOK_None;
+
+  // ... and contain LLVM bitcode
+  for (const char *Filename : Input.second->getValues()) {
+    auto Buff = llvm::MemoryBuffer::getFile(Filename);
+    if (Buff) {
+      auto R = (*Buff)->getMemBufferRef();
+      Expected<llvm::BitcodeLTOInfo> LTOInfo = llvm::getBitcodeLTOInfo(R);
+
+      if (LTOInfo)
+        return LTOInfo->IsThinLTO ? LTOK_Thin : LTOK_Full;
+
+      llvm::consumeError(LTOInfo.takeError());
+    }
+  }
+
+  return LTOK_None;
+}
+
+void Driver::setLTOModeFromInputFiles(const InputList *Inputs) {
+  if (LTOMode != LTOK_None)
+    return;
+
+  for (auto Input : *Inputs) {
+    auto InputLTOMode = getInputLTOMode(Input);
+
+    if (InputLTOMode != LTOK_None) {
+      LTOMode = InputLTOMode;
+      break;
+    }
+  }
+}
+
 // Parse the LTO options and record the type of LTO compilation
 // based on which -f(no-)?lto(=.*)? option occurs last.
 void Driver::setLTOMode(const llvm::opt::ArgList &Args) {
   LTOMode = LTOK_None;
   if (!Args.hasFlag(options::OPT_flto, options::OPT_flto_EQ,
                     options::OPT_fno_lto, false))
     return;
 
Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
 
   if (!HandleImmediateArgs(*C))
     return C;
 
   // Construct the list of inputs.
   InputList Inputs;
   BuildInputs(C->getDefaultToolChain(), *TranslatedArgs, Inputs);
 
+  setLTOModeFromInputFiles(&Inputs);
+
   // Populate the tool chains for the offloading devices, if any.
   CreateOffloadingDeviceToolChains(*C, Inputs);
 
   // Construct the list of abstract actions to perform for this compilation. On
   // MachO targets this uses the driver-driver and universal actions.
   if (TC.getTriple().isOSBinFormatMachO())
     BuildUniversalActions(*C, C->getDefaultToolChain(), Inputs);
   else

clang/test/Driver/thinlto_backend.c

 // CHECK-SAVE-TEMPS-CWD: -save-temps=cwd
 // CHECK-SAVE-TEMPS-OBJ: -save-temps=obj
 // CHECK-SAVE-TEMPS-NOT: -emit-llvm-bc
 
 // Ensure clang driver gives the expected error for incorrect input type
 // RUN: not %clang -O2 -o %t1.o %s -c -fthinlto-index=%t.thinlto.bc 2>&1 \
 // RUN:     | FileCheck %s -check-prefix=CHECK-WARNING
 // CHECK-WARNING: error: option '-fthinlto-index={{.*}}' requires input to be LLVM bitcode
+
+// thinlto enabled without -flto=thin explicitly passed
+// RUN: %clang -O2 %s -c -flto=thin -o %r.o
+// RUN: %clang %r.o -### 2>&1 | FileCheck %s -check-prefix=CHECK-THIN-LTO-ENABLED
+// CHECK-THIN-LTO-ENABLED: -plugin-opt=thinlto
+
+// Same for full LTO
+// RUN: %clang -O2 %s -target x86_64-unknown-linux-gnu -c -flto=full -o %r.o
+// RUN: %clang %r.o -### 2>&1 | FileCheck %s -check-prefix=CHECK-FULL-LTO-ENABLED
+// CHECK-FULL-LTO-ENABLED: LLVMgold