Rollup merge of #53950 - michaelwoerister:more-lto-cli, r=alexcrichton

Allow for opting out of ThinLTO and clean up LTO related cli flag handling.

It turns out that there currently is no way to explicitly disable ThinLTO (except for the nightly-only `-Zthinlto` flag). This PR extends `-C lto` to take `yes` and `no` in addition to `thin` and `fat`. It should be backwards compatible.

It also cleans up how LTO mode selection is handled.

Note that merging the PR in the current state would make the new values for `-C lto` available on the stable channel. I think that would be fine but maybe some team should vote on it.

Author: kennytm

src/librustc/session/config.rs

@@ -68,15 +68,13 @@ pub enum OptLevel {
     SizeMin,    // -Oz
 }
 
+/// This is what the `LtoCli` values get mapped to after resolving defaults and
+/// and taking other command line options into account.
 #[derive(Clone, Copy, PartialEq, Hash, Debug)]
 pub enum Lto {
     /// Don't do any LTO whatsoever
     No,
 
-    /// Do a full crate graph LTO. The flavor is determined by the compiler
-    /// (currently the default is "fat").
-    Yes,
-
     /// Do a full crate graph LTO with ThinLTO
     Thin,
 
@@ -88,6 +86,23 @@ pub enum Lto {
     Fat,
 }
 
+/// The different settings that the `-C lto` flag can have.
+#[derive(Clone, Copy, PartialEq, Hash, Debug)]
+pub enum LtoCli {
+    /// `-C lto=no`
+    No,
+    /// `-C lto=yes`
+    Yes,
+    /// `-C lto`
+    NoParam,
+    /// `-C lto=thin`
+    Thin,
+    /// `-C lto=fat`
+    Fat,
+    /// No `-C lto` flag passed
+    Unspecified,
+}
+
 #[derive(Clone, PartialEq, Hash)]
 pub enum CrossLangLto {
     LinkerPlugin(PathBuf),
@@ -801,15 +816,16 @@ macro_rules! options {
         pub const parse_unpretty: Option<&'static str> =
             Some("`string` or `string=string`");
         pub const parse_lto: Option<&'static str> =
-            Some("one of `thin`, `fat`, or omitted");
+            Some("either a boolean (`yes`, `no`, `on`, `off`, etc), `thin`, \
+                  `fat`, or omitted");
         pub const parse_cross_lang_lto: Option<&'static str> =
             Some("either a boolean (`yes`, `no`, `on`, `off`, etc), \
                   or the path to the linker plugin");
     }
 
     #[allow(dead_code)]
     mod $mod_set {
-        use super::{$struct_name, Passes, Sanitizer, Lto, CrossLangLto};
+        use super::{$struct_name, Passes, Sanitizer, LtoCli, CrossLangLto};
         use rustc_target::spec::{LinkerFlavor, PanicStrategy, RelroLevel};
         use std::path::PathBuf;
 
@@ -1002,11 +1018,23 @@ macro_rules! options {
             }
         }
 
-        fn parse_lto(slot: &mut Lto, v: Option<&str>) -> bool {
+        fn parse_lto(slot: &mut LtoCli, v: Option<&str>) -> bool {
+            if v.is_some() {
+                let mut bool_arg = None;
+                if parse_opt_bool(&mut bool_arg, v) {
+                    *slot = if bool_arg.unwrap() {
+                        LtoCli::Yes
+                    } else {
+                        LtoCli::No
+                    };
+                    return true
+                }
+            }
+
             *slot = match v {
-                None => Lto::Yes,
-                Some("thin") => Lto::Thin,
-                Some("fat") => Lto::Fat,
+                None => LtoCli::NoParam,
+                Some("thin") => LtoCli::Thin,
+                Some("fat") => LtoCli::Fat,
                 Some(_) => return false,
             };
             true
@@ -1047,7 +1075,7 @@ options! {CodegenOptions, CodegenSetter, basic_codegen_options,
         "extra arguments to append to the linker invocation (space separated)"),
     link_dead_code: bool = (false, parse_bool, [UNTRACKED],
         "don't let linker strip dead code (turning it on can be used for code coverage)"),
-    lto: Lto = (Lto::No, parse_lto, [TRACKED],
+    lto: LtoCli = (LtoCli::Unspecified, parse_lto, [TRACKED],
         "perform LLVM link-time optimizations"),
     target_cpu: Option<String> = (None, parse_opt_string, [TRACKED],
         "select target processor (rustc --print target-cpus for details)"),
@@ -2384,8 +2412,8 @@ mod dep_tracking {
     use std::hash::Hash;
     use std::path::PathBuf;
     use std::collections::hash_map::DefaultHasher;
-    use super::{CrateType, DebugInfo, ErrorOutputType, Lto, OptLevel, OutputTypes,
-                Passes, Sanitizer, CrossLangLto};
+    use super::{CrateType, DebugInfo, ErrorOutputType, OptLevel, OutputTypes,
+                Passes, Sanitizer, LtoCli, CrossLangLto};
     use syntax::feature_gate::UnstableFeatures;
     use rustc_target::spec::{PanicStrategy, RelroLevel, TargetTriple};
     use syntax::edition::Edition;
@@ -2440,7 +2468,7 @@ mod dep_tracking {
     impl_dep_tracking_hash_via_hash!(RelroLevel);
     impl_dep_tracking_hash_via_hash!(Passes);
     impl_dep_tracking_hash_via_hash!(OptLevel);
-    impl_dep_tracking_hash_via_hash!(Lto);
+    impl_dep_tracking_hash_via_hash!(LtoCli);
     impl_dep_tracking_hash_via_hash!(DebugInfo);
     impl_dep_tracking_hash_via_hash!(UnstableFeatures);
     impl_dep_tracking_hash_via_hash!(OutputTypes);
@@ -2514,7 +2542,7 @@ mod tests {
     use lint;
     use middle::cstore;
     use session::config::{build_configuration, build_session_options_and_crate_config};
-    use session::config::{Lto, CrossLangLto};
+    use session::config::{LtoCli, CrossLangLto};
     use session::build_session;
     use std::collections::{BTreeMap, BTreeSet};
     use std::iter::FromIterator;
@@ -2948,7 +2976,7 @@ mod tests {
 
         // Make sure changing a [TRACKED] option changes the hash
         opts = reference.clone();
-        opts.cg.lto = Lto::Fat;
+        opts.cg.lto = LtoCli::Fat;
         assert!(reference.dep_tracking_hash() != opts.dep_tracking_hash());
 
         opts = reference.clone();

src/librustc/session/mod.rs

@@ -550,9 +550,27 @@ impl Session {
         // lto` and we've for whatever reason forced off ThinLTO via the CLI,
         // then ensure we can't use a ThinLTO.
         match self.opts.cg.lto {
-            config::Lto::No => {}
-            config::Lto::Yes if self.opts.cli_forced_thinlto_off => return config::Lto::Fat,
-            other => return other,
+            config::LtoCli::Unspecified => {
+                // The compiler was invoked without the `-Clto` flag. Fall
+                // through to the default handling
+            }
+            config::LtoCli::No => {
+                // The user explicitly opted out of any kind of LTO
+                return config::Lto::No;
+            }
+            config::LtoCli::Yes |
+            config::LtoCli::Fat |
+            config::LtoCli::NoParam => {
+                // All of these mean fat LTO
+                return config::Lto::Fat;
+            }
+            config::LtoCli::Thin => {
+                return if self.opts.cli_forced_thinlto_off {
+                    config::Lto::Fat
+                } else {
+                    config::Lto::Thin
+                };
+            }
         }
 
         // Ok at this point the target doesn't require anything and the user
@@ -1178,7 +1196,6 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
 
     if sess.opts.incremental.is_some() {
         match sess.lto() {
-            Lto::Yes |
             Lto::Thin |
             Lto::Fat => {
                 sess.err("can't perform LTO when compiling incrementally");

src/librustc_codegen_llvm/back/link.rs

@@ -1666,7 +1666,6 @@ fn relevant_lib(sess: &Session, lib: &NativeLibrary) -> bool {
 
 fn are_upstream_rust_objects_already_included(sess: &Session) -> bool {
     match sess.lto() {
-        Lto::Yes |
         Lto::Fat => true,
         Lto::Thin => {
             // If we defer LTO to the linker, we haven't run LTO ourselves, so

src/librustc_codegen_llvm/back/linker.rs

@@ -205,13 +205,12 @@ impl<'a> GccLinker<'a> {
         self.linker_arg(&format!("-plugin-opt={}", opt_level));
         self.linker_arg(&format!("-plugin-opt=mcpu={}", llvm_util::target_cpu(self.sess)));
 
-        match self.sess.opts.cg.lto {
+        match self.sess.lto() {
             config::Lto::Thin |
             config::Lto::ThinLocal => {
                 self.linker_arg("-plugin-opt=thin");
             }
             config::Lto::Fat |
-            config::Lto::Yes |
             config::Lto::No => {
                 // default to regular LTO
             }

src/librustc_codegen_llvm/back/lto.rs

@@ -118,7 +118,7 @@ pub(crate) fn run(cgcx: &CodegenContext,
         Lto::ThinLocal => SymbolExportLevel::Rust,
 
         // We're doing LTO for the entire crate graph
-        Lto::Yes | Lto::Fat | Lto::Thin => {
+        Lto::Fat | Lto::Thin => {
             symbol_export::crates_export_threshold(&cgcx.crate_types)
         }
 
@@ -201,7 +201,6 @@ pub(crate) fn run(cgcx: &CodegenContext,
                                              .map(|c| c.as_ptr())
                                              .collect::<Vec<_>>();
     match cgcx.lto {
-        Lto::Yes | // `-C lto` == fat LTO by default
         Lto::Fat => {
             assert!(cached_modules.is_empty());
             let opt_jobs = fat_lto(cgcx,

src/librustc_codegen_llvm/back/write.rs

@@ -937,7 +937,6 @@ fn need_pre_thin_lto_bitcode_for_incr_comp(sess: &Session) -> bool {
     }
 
     match sess.lto() {
-        Lto::Yes |
         Lto::Fat |
         Lto::No => false,
         Lto::Thin |
@@ -1372,7 +1371,7 @@ fn execute_optimize_work_item(cgcx: &CodegenContext,
         // require LTO so the request for LTO is always unconditionally
         // passed down to the backend, but we don't actually want to do
         // anything about it yet until we've got a final product.
-        Lto::Yes | Lto::Fat | Lto::Thin => {
+        Lto::Fat | Lto::Thin => {
             cgcx.crate_types.len() != 1 ||
                 cgcx.crate_types[0] != config::CrateType::Rlib
         }
@@ -1552,7 +1551,7 @@ fn start_executing_work(tcx: TyCtxt,
                 exported_symbols.insert(LOCAL_CRATE, copy_symbols(LOCAL_CRATE));
                 Some(Arc::new(exported_symbols))
             }
-            Lto::Yes | Lto::Fat | Lto::Thin => {
+            Lto::Fat | Lto::Thin => {
                 exported_symbols.insert(LOCAL_CRATE, copy_symbols(LOCAL_CRATE));
                 for &cnum in tcx.crates().iter() {
                     exported_symbols.insert(cnum, copy_symbols(cnum));

src/test/run-make-fulldeps/codegen-options-parsing/Makefile

@@ -16,11 +16,11 @@ all:
 	$(RUSTC) -C extra-filename=foo dummy.rs 2>&1
 	#Option taking no argument
 	$(RUSTC) -C lto= dummy.rs 2>&1 | \
-		$(CGREP) 'codegen option `lto` - one of `thin`, `fat`, or'
+		$(CGREP) 'codegen option `lto` - either a boolean (`yes`, `no`, `on`, `off`, etc), `thin`, `fat`, or omitted'
 	$(RUSTC) -C lto=1 dummy.rs 2>&1 | \
-		$(CGREP) 'codegen option `lto` - one of `thin`, `fat`, or'
+		$(CGREP) 'codegen option `lto` - either a boolean (`yes`, `no`, `on`, `off`, etc), `thin`, `fat`, or omitted'
 	$(RUSTC) -C lto=foo dummy.rs 2>&1 | \
-		$(CGREP) 'codegen option `lto` - one of `thin`, `fat`, or'
+		$(CGREP) 'codegen option `lto` - either a boolean (`yes`, `no`, `on`, `off`, etc), `thin`, `fat`, or omitted'
 	$(RUSTC) -C lto dummy.rs
 
 	# Should not link dead code...

src/test/run-make-fulldeps/lto-smoke/Makefile

@@ -1,6 +1,30 @@
 -include ../tools.mk
 
-all:
+all: noparam bool_true bool_false thin fat
+
+noparam:
 	$(RUSTC) lib.rs
 	$(RUSTC) main.rs -C lto
 	$(call RUN,main)
+
+bool_true:
+	$(RUSTC) lib.rs
+	$(RUSTC) main.rs -C lto=yes
+	$(call RUN,main)
+
+
+bool_false:
+	$(RUSTC) lib.rs
+	$(RUSTC) main.rs -C lto=off
+	$(call RUN,main)
+
+thin:
+	$(RUSTC) lib.rs
+	$(RUSTC) main.rs -C lto=thin
+	$(call RUN,main)
+
+fat:
+	$(RUSTC) lib.rs
+	$(RUSTC) main.rs -C lto=fat
+	$(call RUN,main)
+