[clang][ThinLTO][Legacy] Don't add -fsplit-lto-unit for legacy thin LTO builds

[evgeny777]

Summary

This is an addition to D68950 which has been recently landed. It prevents adding -fsplit-lto-unit for Mac and PS4 thin LTO builds, because legacy LTO library is not capable of unit splitting.

[steven_wu]

A small suggestion for improvement.

lib/Driver/ToolChains/Clang.cpp

5400 ↗

(On Diff #225638)

I think this would be cleaner if we add a virtual function supportSplitLTOUnit to Toolchain, and let toolchain decide if it is supported, rather than hard code this in Clang.cpp.

[evgeny777]

Addressed

lib/Driver/ToolChains/Clang.cpp

5400 ↗

(On Diff #225638)

It doesn't make sense to use virtual function, because there is no toolchain class for PS4. I'd suggest make an ordinary member function checking triple

[steven_wu]

That is not exactly what I meant. I meant:

in Toolchain.h: virtual bool canSplitThinLTOUnit() const { return true; }
in lib/lib/Driver/ToolChains/{PS4CPU.h,Darwin.h}: return false; // Using legacy LTO library

[steven_wu]

lib/Driver/ToolChains/Clang.cpp

5400 ↗

(On Diff #225638)

PS4CPU.h is not PS4 toolchain? If there is no PS4 toolchain, let's stick with your original patch.

[evgeny777]

lib/Driver/ToolChains/Clang.cpp

5400 ↗

(On Diff #225638)

Hm... I somehow didn't realized that PS4CPU is derived from ToolChain. I'll update the patch

[evgeny777]

Addressed

[steven_wu]

LGTM

clang/include/clang/Driver/ToolChain.h

public:
 
   /// Return sanitizers which are available in this toolchain.
   virtual SanitizerMask getSupportedSanitizers() const;
 
   /// Return sanitizers which are enabled by default.
   virtual SanitizerMask getDefaultSanitizers() const {
     return SanitizerMask();
   }
+
+  /// Returns true when it's possible to split LTO unit to use whole
+  /// program devirtualization and CFI santiizers.
+  virtual bool canSplitThinLTOUnit() const { return true; }
 };
 
 /// Set a ToolChain's effective triple. Reset it when the registration object
 /// is destroyed.
 class RegisterEffectiveTriple {
   const ToolChain &TC;
 
 public:

clang/lib/Driver/ToolChains/Clang.cpp

   if (WholeProgramVTables) {
     if (!D.isUsingLTO())
       D.Diag(diag::err_drv_argument_only_allowed_with)
           << "-fwhole-program-vtables"
           << "-flto";
     CmdArgs.push_back("-fwhole-program-vtables");
   }
 
-  bool DefaultsSplitLTOUnit = WholeProgramVTables || Sanitize.needsLTO();
+  bool DefaultsSplitLTOUnit =
+      (WholeProgramVTables || Sanitize.needsLTO()) &&
+      (D.getLTOMode() == LTOK_Full || TC.canSplitThinLTOUnit());
   bool SplitLTOUnit =
       Args.hasFlag(options::OPT_fsplit_lto_unit,
                    options::OPT_fno_split_lto_unit, DefaultsSplitLTOUnit);
   if (Sanitize.needsLTO() && !SplitLTOUnit)
     D.Diag(diag::err_drv_argument_not_allowed_with) << "-fno-split-lto-unit"
                                                     << "-fsanitize=cfi";
   if (SplitLTOUnit)
     CmdArgs.push_back("-fsplit-lto-unit");

clang/lib/Driver/ToolChains/Darwin.h

public:
   GetExceptionModel(const llvm::opt::ArgList &Args) const override {
     return llvm::ExceptionHandling::None;
   }
 
   virtual StringRef getOSLibraryNameSuffix(bool IgnoreSim = false) const {
     return "";
   }
 
+  // Darwin toolchain uses legacy thin LTO API, which is not
+  // capable of unit splitting.
+  bool canSplitThinLTOUnit() const override { return false; }
   /// }
 };
 
 /// Darwin - The base Darwin tool chain.
 class LLVM_LIBRARY_VISIBILITY Darwin : public MachO {
 public:
   /// Whether the information on the target has been initialized.
   //

clang/lib/Driver/ToolChains/PS4CPU.h

public:
   }
 
   llvm::DebuggerKind getDefaultDebuggerTuning() const override {
     return llvm::DebuggerKind::SCE;
   }
 
   SanitizerMask getSupportedSanitizers() const override;
 
+  // PS4 toolchain uses legacy thin LTO API, which is not
+  // capable of unit splitting.
+  bool canSplitThinLTOUnit() const override { return false; }
+
 protected:
   Tool *buildAssembler() const override;
   Tool *buildLinker() const override;
 };
 
 } // end namespace toolchains
 } // end namespace driver
 } // end namespace clang
 
 #endif // LLVM_CLANG_LIB_DRIVER_TOOLCHAINS_PS4CPU_H

clang/test/Driver/split-lto-unit.c

 // RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin 2>&1 | FileCheck --check-prefix=NOUNIT %s
 // RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fsplit-lto-unit 2>&1 | FileCheck --check-prefix=UNIT %s
 // RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fno-split-lto-unit 2>&1 | FileCheck --check-prefix=NOUNIT %s
 // RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fno-split-lto-unit -fwhole-program-vtables 2>&1 | FileCheck --check-prefix=ERROR1 %s
 // RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fno-split-lto-unit -fsanitize=cfi 2>&1 | FileCheck --check-prefix=ERROR2 %s
+// RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -fwhole-program-vtables -flto=full 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -fwhole-program-vtables -flto=thin 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang -target x86_64-scei-ps4 -### %s -fwhole-program-vtables -flto=full 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-scei-ps4 -### %s -fwhole-program-vtables -flto=thin 2>&1 | FileCheck --check-prefix=NOUNIT %s
 
 // UNIT: "-fsplit-lto-unit"
 // NOUNIT-NOT: "-fsplit-lto-unit"
 // ERROR1-NOT: error: invalid argument
 // ERROR2: error: invalid argument '-fno-split-lto-unit' not allowed with '-fsanitize=cfi'