tejohnson updated this revision to Diff 172181.
tejohnson added a comment.

Address comments:
Promote -flto-unit to clang driver option (and test it)
Adjust LTOVisibility.rst to reflect change of default and new option.


Repository:
  rC Clang

https://reviews.llvm.org/D53524

Files:
  docs/LTOVisibility.rst
  include/clang/Driver/CC1Options.td
  include/clang/Driver/Options.td
  include/clang/Driver/SanitizerArgs.h
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/Clang.cpp
  test/Driver/lto-unit.c

Index: test/Driver/lto-unit.c
===================================================================
--- test/Driver/lto-unit.c
+++ test/Driver/lto-unit.c
@@ -1,9 +1,18 @@
-// RUN: %clang -target x86_64-unknown-linux -### %s -flto=full 2>&1 | FileCheck --check-prefix=UNIT %s
-// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin 2>&1 | FileCheck --check-prefix=UNIT %s
-// RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=full 2>&1 | FileCheck --check-prefix=UNIT %s
-// RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=thin 2>&1 | FileCheck --check-prefix=NOUNIT %s
-// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=full 2>&1 | FileCheck --check-prefix=UNIT %s
-// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=thin 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=full -fwhole-program-vtables 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -fwhole-program-vtables 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=full -flto-unit 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto=thin -flto-unit 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=full -fwhole-program-vtables 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=thin -fwhole-program-vtables 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=full -flto-unit 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-apple-darwin13.3.0 -### %s -flto=thin -flto-unit 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=full -fwhole-program-vtables 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=thin -fwhole-program-vtables 2>&1 | FileCheck --check-prefix=NOUNIT %s
+// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=full -flto-unit 2>&1 | FileCheck --check-prefix=UNIT %s
+// RUN: %clang -target x86_64-scei-ps4 -### %s -flto=thin -flto-unit 2>&1 | FileCheck --check-prefix=NOUNIT %s
 
 // UNIT: "-flto-unit"
 // NOUNIT-NOT: "-flto-unit"
+
+// RUN: %clang -target x86_64-unknown-linux -### %s -flto-unit 2>&1 | FileCheck --check-prefix=NO-LTO %s
+// NO-LTO: invalid argument '-flto-unit' only allowed with '-flto'
Index: lib/Driver/ToolChains/Clang.cpp
===================================================================
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -3364,21 +3364,6 @@
     // the use-list order, since serialization to bitcode is part of the flow.
     if (JA.getType() == types::TY_LLVM_BC)
       CmdArgs.push_back("-emit-llvm-uselists");
-
-    // Device-side jobs do not support LTO.
-    bool isDeviceOffloadAction = !(JA.isDeviceOffloading(Action::OFK_None) ||
-                                   JA.isDeviceOffloading(Action::OFK_Host));
-
-    if (D.isUsingLTO() && !isDeviceOffloadAction) {
-      Args.AddLastArg(CmdArgs, options::OPT_flto, options::OPT_flto_EQ);
-
-      // The Darwin and PS4 linkers currently use the legacy LTO API, which
-      // does not support LTO unit features (CFI, whole program vtable opt)
-      // under ThinLTO.
-      if (!(RawTriple.isOSDarwin() || RawTriple.isPS4()) ||
-          D.getLTOMode() == LTOK_Full)
-        CmdArgs.push_back("-flto-unit");
-    }
   }
 
   if (const Arg *A = Args.getLastArg(options::OPT_fthinlto_index_EQ)) {
@@ -4980,6 +4965,31 @@
     CmdArgs.push_back("-fwhole-program-vtables");
   }
 
+  bool LTOUnit = Args.hasArg(options::OPT_flto_unit);
+  if (LTOUnit && !D.isUsingLTO())
+    D.Diag(diag::err_drv_argument_only_allowed_with) << "-flto-unit"
+                                                     << "-flto";
+
+  // Device-side jobs do not support LTO.
+  bool isDeviceOffloadAction = !(JA.isDeviceOffloading(Action::OFK_None) ||
+                                 JA.isDeviceOffloading(Action::OFK_Host));
+
+  if (D.isUsingLTO() &&
+      (isa<CompileJobAction>(JA) || isa<BackendJobAction>(JA)) &&
+      !isDeviceOffloadAction) {
+    Args.AddLastArg(CmdArgs, options::OPT_flto, options::OPT_flto_EQ);
+
+    // Enable LTO unit if need for CFI or whole program vtable optimization.
+    // The Darwin and PS4 linkers currently use the legacy LTO API, which
+    // does not support LTO unit features (CFI, whole program vtable opt)
+    // under ThinLTO.
+    bool SupportsLTOUnit = !(RawTriple.isOSDarwin() || RawTriple.isPS4()) ||
+                           D.getLTOMode() == LTOK_Full;
+    if ((LTOUnit || WholeProgramVTables || Sanitize.needsLTO()) &&
+        SupportsLTOUnit)
+      CmdArgs.push_back("-flto-unit");
+  }
+
   if (Arg *A = Args.getLastArg(options::OPT_fexperimental_isel,
                                options::OPT_fno_experimental_isel)) {
     CmdArgs.push_back("-mllvm");
Index: lib/Driver/SanitizerArgs.cpp
===================================================================
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -207,6 +207,10 @@
   return Sanitizers.Mask & NeedsUnwindTables;
 }
 
+bool SanitizerArgs::needsLTO() const {
+  return Sanitizers.Mask & NeedsLTO;
+}
+
 SanitizerArgs::SanitizerArgs(const ToolChain &TC,
                              const llvm::opt::ArgList &Args) {
   SanitizerMask AllRemove = 0;  // During the loop below, the accumulated set of
Index: include/clang/Driver/SanitizerArgs.h
===================================================================
--- include/clang/Driver/SanitizerArgs.h
+++ include/clang/Driver/SanitizerArgs.h
@@ -78,6 +78,7 @@
 
   bool requiresPIE() const;
   bool needsUnwindTables() const;
+  bool needsLTO() const;
   bool linkCXXRuntimes() const { return LinkCXXRuntimes; }
   bool hasCrossDsoCfi() const { return CfiCrossDso; }
   void addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args,
Index: include/clang/Driver/Options.td
===================================================================
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -1727,6 +1727,9 @@
   HelpText<"Enables whole-program vtable optimization. Requires -flto">;
 def fno_whole_program_vtables : Flag<["-"], "fno-whole-program-vtables">, Group<f_Group>,
   Flags<[CoreOption]>;
+def flto_unit: Flag<["-"], "flto-unit">, Group<f_Group>,
+  Flags<[CoreOption, CC1Option]>,
+  HelpText<"Emit IR to support LTO unit features (CFI, whole program vtable opt)">;
 def fforce_emit_vtables : Flag<["-"], "fforce-emit-vtables">, Group<f_Group>,
     Flags<[CC1Option]>,
     HelpText<"Emits more virtual tables to improve devirtualization">;
Index: include/clang/Driver/CC1Options.td
===================================================================
--- include/clang/Driver/CC1Options.td
+++ include/clang/Driver/CC1Options.td
@@ -345,8 +345,6 @@
 def flto_visibility_public_std:
     Flag<["-"], "flto-visibility-public-std">,
     HelpText<"Use public LTO visibility for classes in std and stdext namespaces">;
-def flto_unit: Flag<["-"], "flto-unit">,
-    HelpText<"Emit IR to support LTO unit features (CFI, whole program vtable opt)">;
 def fno_lto_unit: Flag<["-"], "fno-lto-unit">;
 def fthin_link_bitcode_EQ : Joined<["-"], "fthin-link-bitcode=">,
     HelpText<"Write minimized bitcode to <file> for the ThinLTO thin link only">;
Index: docs/LTOVisibility.rst
===================================================================
--- docs/LTOVisibility.rst
+++ docs/LTOVisibility.rst
@@ -6,8 +6,14 @@
 referenced from outside the current LTO unit. A *linkage unit* is a set of
 translation units linked together into an executable or DSO, and a linkage
 unit's *LTO unit* is the subset of the linkage unit that is linked together
-using link-time optimization; in the case where LTO is not being used, the
-linkage unit's LTO unit is empty. Each linkage unit has only a single LTO unit.
+using link-time optimization; in the case where LTO units are not being used,
+the linkage unit's LTO unit is empty. Each linkage unit has only a single LTO
+unit.
+
+LTO units are produced during LTO compiles when also compiling with
+``-fwhole-program-vtables`` or control flow integrity (e.g.
+``-fsanitize=cfi-vcall`` and ``-fsanitize=cfi-mfcall``), or when LTO units
+are explicitly enabled (``-flto-unit``).
 
 The LTO visibility of a class is used by the compiler to determine which
 classes the whole-program devirtualization (``-fwhole-program-vtables``) and
@@ -24,7 +30,7 @@
 visibility. A class's LTO visibility is treated as an ODR-relevant property
 of its definition, so it must be consistent between translation units.
 
-In translation units built with LTO, LTO visibility is based on the
+In translation units built with LTO units, LTO visibility is based on the
 class's symbol visibility as expressed at the source level (i.e. the
 ``__attribute__((visibility("...")))`` attribute, or the ``-fvisibility=``
 flag) or, on the Windows platform, the dllimport and dllexport attributes. When
@@ -35,16 +41,16 @@
 (e.g. classes declared in unnamed namespaces) also receive hidden LTO
 visibility.
 
-A class defined in a translation unit built without LTO receives public
+A class defined in a translation unit built without LTO units receives public
 LTO visibility regardless of its object file visibility, linkage or other
 attributes.
 
 This mechanism will produce the correct result in most cases, but there are
 two cases where it may wrongly infer hidden LTO visibility.
 
 1. As a corollary of the above rules, if a linkage unit is produced from a
-   combination of LTO object files and non-LTO object files, any hidden
-   visibility class defined in both a translation unit built with LTO and
+   combination of LTO unit object files and non-LTO unit object files, any
+   hidden visibility class defined in both a translation unit built with LTO and
    a translation unit built without LTO must be defined with public LTO
    visibility in order to avoid an ODR violation.
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to