From 7b1df42accbfdf54148072c7a3d1e80177c388b0 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 10 Apr 2019 13:46:37 +0200 Subject: [PATCH] Clean up handling of -Zpgo-gen commandline option. --- src/librustc/session/config.rs | 38 ++++++++++++++++--- src/librustc_codegen_llvm/attributes.rs | 2 +- src/librustc_codegen_llvm/back/link.rs | 2 +- src/librustc_codegen_llvm/back/write.rs | 22 ++++++++--- .../back/symbol_export.rs | 2 +- src/librustc_codegen_ssa/back/write.rs | 7 ++-- src/librustc_metadata/creader.rs | 2 +- .../run-make-fulldeps/pgo-gen-lto/Makefile | 6 +-- src/test/run-make-fulldeps/pgo-gen/Makefile | 6 +-- 9 files changed, 60 insertions(+), 27 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 988e4a9ff23a2..7c0eab26b09b6 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -113,6 +113,21 @@ impl LinkerPluginLto { } } +#[derive(Clone, PartialEq, Hash)] +pub enum PgoGenerate { + Enabled(Option), + Disabled, +} + +impl PgoGenerate { + pub fn enabled(&self) -> bool { + match *self { + PgoGenerate::Enabled(_) => true, + PgoGenerate::Disabled => false, + } + } +} + #[derive(Clone, Copy, PartialEq, Hash)] pub enum DebugInfo { None, @@ -826,13 +841,15 @@ macro_rules! options { pub const parse_linker_plugin_lto: Option<&str> = Some("either a boolean (`yes`, `no`, `on`, `off`, etc), \ or the path to the linker plugin"); + pub const parse_pgo_generate: Option<&str> = + Some("an optional path to the profiling data output directory"); pub const parse_merge_functions: Option<&str> = Some("one of: `disabled`, `trampolines`, or `aliases`"); } #[allow(dead_code)] mod $mod_set { - use super::{$struct_name, Passes, Sanitizer, LtoCli, LinkerPluginLto}; + use super::{$struct_name, Passes, Sanitizer, LtoCli, LinkerPluginLto, PgoGenerate}; use rustc_target::spec::{LinkerFlavor, MergeFunctions, PanicStrategy, RelroLevel}; use std::path::PathBuf; use std::str::FromStr; @@ -1087,6 +1104,14 @@ macro_rules! options { true } + fn parse_pgo_generate(slot: &mut PgoGenerate, v: Option<&str>) -> bool { + *slot = match v { + None => PgoGenerate::Enabled(None), + Some(path) => PgoGenerate::Enabled(Some(PathBuf::from(path))), + }; + true + } + fn parse_merge_functions(slot: &mut Option, v: Option<&str>) -> bool { match v.and_then(|s| MergeFunctions::from_str(s).ok()) { Some(mergefunc) => *slot = Some(mergefunc), @@ -1363,7 +1388,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options, "extra arguments to prepend to the linker invocation (space separated)"), profile: bool = (false, parse_bool, [TRACKED], "insert profiling code"), - pgo_gen: Option = (None, parse_opt_string, [TRACKED], + pgo_gen: PgoGenerate = (PgoGenerate::Disabled, parse_pgo_generate, [TRACKED], "Generate PGO profile data, to a given file, or to the default location if it's empty."), pgo_use: String = (String::new(), parse_string, [TRACKED], "Use PGO profile data from the given profile file."), @@ -1980,7 +2005,7 @@ pub fn build_session_options_and_crate_config( ); } - if debugging_opts.pgo_gen.is_some() && !debugging_opts.pgo_use.is_empty() { + if debugging_opts.pgo_gen.enabled() && !debugging_opts.pgo_use.is_empty() { early_error( error_format, "options `-Z pgo-gen` and `-Z pgo-use` are exclusive", @@ -2490,7 +2515,7 @@ mod dep_tracking { use std::path::PathBuf; use std::collections::hash_map::DefaultHasher; use super::{CrateType, DebugInfo, ErrorOutputType, OptLevel, OutputTypes, - Passes, Sanitizer, LtoCli, LinkerPluginLto}; + Passes, Sanitizer, LtoCli, LinkerPluginLto, PgoGenerate}; use syntax::feature_gate::UnstableFeatures; use rustc_target::spec::{MergeFunctions, PanicStrategy, RelroLevel, TargetTriple}; use syntax::edition::Edition; @@ -2558,6 +2583,7 @@ mod dep_tracking { impl_dep_tracking_hash_via_hash!(TargetTriple); impl_dep_tracking_hash_via_hash!(Edition); impl_dep_tracking_hash_via_hash!(LinkerPluginLto); + impl_dep_tracking_hash_via_hash!(PgoGenerate); impl_dep_tracking_hash_for_sortable_vec_of!(String); impl_dep_tracking_hash_for_sortable_vec_of!(PathBuf); @@ -2625,7 +2651,7 @@ mod tests { build_session_options_and_crate_config, to_crate_config }; - use crate::session::config::{LtoCli, LinkerPluginLto}; + use crate::session::config::{LtoCli, LinkerPluginLto, PgoGenerate}; use crate::session::build_session; use crate::session::search_paths::SearchPath; use std::collections::{BTreeMap, BTreeSet}; @@ -3124,7 +3150,7 @@ mod tests { assert!(reference.dep_tracking_hash() != opts.dep_tracking_hash()); opts = reference.clone(); - opts.debugging_opts.pgo_gen = Some(String::from("abc")); + opts.debugging_opts.pgo_gen = PgoGenerate::Enabled(None); assert_ne!(reference.dep_tracking_hash(), opts.dep_tracking_hash()); opts = reference.clone(); diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index 77fa34e74dd70..b15a64c966b1b 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/src/librustc_codegen_llvm/attributes.rs @@ -104,7 +104,7 @@ pub fn set_probestack(cx: &CodegenCx<'ll, '_>, llfn: &'ll Value) { } // probestack doesn't play nice either with pgo-gen. - if cx.sess().opts.debugging_opts.pgo_gen.is_some() { + if cx.sess().opts.debugging_opts.pgo_gen.enabled() { return; } diff --git a/src/librustc_codegen_llvm/back/link.rs b/src/librustc_codegen_llvm/back/link.rs index 19419a72b94dd..6a3c2adc85618 100644 --- a/src/librustc_codegen_llvm/back/link.rs +++ b/src/librustc_codegen_llvm/back/link.rs @@ -1014,7 +1014,7 @@ fn link_args(cmd: &mut dyn Linker, cmd.build_static_executable(); } - if sess.opts.debugging_opts.pgo_gen.is_some() { + if sess.opts.debugging_opts.pgo_gen.enabled() { cmd.pgo_gen(); } diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index f0ed201ad5c27..4de65325637ff 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -13,7 +13,7 @@ use crate::LlvmCodegenBackend; use rustc::hir::def_id::LOCAL_CRATE; use rustc_codegen_ssa::back::write::{CodegenContext, ModuleConfig, run_assembler}; use rustc_codegen_ssa::traits::*; -use rustc::session::config::{self, OutputType, Passes, Lto}; +use rustc::session::config::{self, OutputType, Passes, Lto, PgoGenerate}; use rustc::session::Session; use rustc::ty::TyCtxt; use rustc_codegen_ssa::{ModuleCodegen, CompiledModule}; @@ -26,7 +26,7 @@ use errors::{Handler, FatalError}; use std::ffi::{CString, CStr}; use std::fs; use std::io::{self, Write}; -use std::path::Path; +use std::path::{Path, PathBuf}; use std::str; use std::sync::Arc; use std::slice; @@ -708,10 +708,20 @@ pub unsafe fn with_llvm_pmb(llmod: &llvm::Module, .unwrap_or(llvm::CodeGenOptSizeNone); let inline_threshold = config.inline_threshold; - let pgo_gen_path = config.pgo_gen.as_ref().map(|s| { - let s = if s.is_empty() { "default_%m.profraw" } else { s }; - CString::new(s.as_bytes()).unwrap() - }); + let pgo_gen_path = match config.pgo_gen { + PgoGenerate::Enabled(ref opt_dir_path) => { + let path = if let Some(dir_path) = opt_dir_path { + dir_path.join("default_%m.profraw") + } else { + PathBuf::from("default_%m.profraw") + }; + + Some(CString::new(format!("{}", path.display())).unwrap()) + } + PgoGenerate::Disabled => { + None + } + }; let pgo_use_path = if config.pgo_use.is_empty() { None diff --git a/src/librustc_codegen_ssa/back/symbol_export.rs b/src/librustc_codegen_ssa/back/symbol_export.rs index 336f41b784a81..a55f783df43a3 100644 --- a/src/librustc_codegen_ssa/back/symbol_export.rs +++ b/src/librustc_codegen_ssa/back/symbol_export.rs @@ -209,7 +209,7 @@ fn exported_symbols_provider_local<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } } - if tcx.sess.opts.debugging_opts.pgo_gen.is_some() { + if tcx.sess.opts.debugging_opts.pgo_gen.enabled() { // These are weak symbols that point to the profile version and the // profile name, which need to be treated as exported so LTO doesn't nix // them. diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index fa8c4177eafe2..7fc43f29701ad 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -12,7 +12,8 @@ use rustc_incremental::{copy_cgu_workproducts_to_incr_comp_cache_dir, use rustc::dep_graph::{WorkProduct, WorkProductId, WorkProductFileKind}; use rustc::dep_graph::cgu_reuse_tracker::CguReuseTracker; use rustc::middle::cstore::EncodedMetadata; -use rustc::session::config::{self, OutputFilenames, OutputType, Passes, Sanitizer, Lto}; +use rustc::session::config::{self, OutputFilenames, OutputType, Passes, Lto, + Sanitizer, PgoGenerate}; use rustc::session::Session; use rustc::util::nodemap::FxHashMap; use rustc::hir::def_id::{CrateNum, LOCAL_CRATE}; @@ -56,7 +57,7 @@ pub struct ModuleConfig { /// Some(level) to optimize binary size, or None to not affect program size. pub opt_size: Option, - pub pgo_gen: Option, + pub pgo_gen: PgoGenerate, pub pgo_use: String, // Flags indicating which outputs to produce. @@ -94,7 +95,7 @@ impl ModuleConfig { opt_level: None, opt_size: None, - pgo_gen: None, + pgo_gen: PgoGenerate::Disabled, pgo_use: String::new(), emit_no_opt_bc: false, diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index 36d9bf9f50dd5..66daa4518bef6 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -862,7 +862,7 @@ impl<'a> CrateLoader<'a> { fn inject_profiler_runtime(&mut self) { if self.sess.opts.debugging_opts.profile || - self.sess.opts.debugging_opts.pgo_gen.is_some() + self.sess.opts.debugging_opts.pgo_gen.enabled() { info!("loading profiler"); diff --git a/src/test/run-make-fulldeps/pgo-gen-lto/Makefile b/src/test/run-make-fulldeps/pgo-gen-lto/Makefile index e358314c0d09e..7c19961b1e420 100644 --- a/src/test/run-make-fulldeps/pgo-gen-lto/Makefile +++ b/src/test/run-make-fulldeps/pgo-gen-lto/Makefile @@ -1,10 +1,8 @@ -include ../tools.mk -# ignore-windows - all: ifeq ($(PROFILER_SUPPORT),1) - $(RUSTC) -Copt-level=3 -Clto=fat -Z pgo-gen="$(TMPDIR)/test.profraw" test.rs + $(RUSTC) -Copt-level=3 -Clto=fat -Z pgo-gen="$(TMPDIR)" test.rs $(call RUN,test) || exit 1 - [ -e "$(TMPDIR)/test.profraw" ] || (echo "No .profraw file"; exit 1) + [ -e "$(TMPDIR)"/default_*.profraw ] || (echo "No .profraw file"; exit 1) endif diff --git a/src/test/run-make-fulldeps/pgo-gen/Makefile b/src/test/run-make-fulldeps/pgo-gen/Makefile index 1961dff8d783e..0469c4443d85a 100644 --- a/src/test/run-make-fulldeps/pgo-gen/Makefile +++ b/src/test/run-make-fulldeps/pgo-gen/Makefile @@ -1,10 +1,8 @@ -include ../tools.mk -# ignore-windows - all: ifeq ($(PROFILER_SUPPORT),1) - $(RUSTC) -g -Z pgo-gen="$(TMPDIR)/test.profraw" test.rs + $(RUSTC) -g -Z pgo-gen="$(TMPDIR)" test.rs $(call RUN,test) || exit 1 - [ -e "$(TMPDIR)/test.profraw" ] || (echo "No .profraw file"; exit 1) + [ -e "$(TMPDIR)"/default_*.profraw ] || (echo "No .profraw file"; exit 1) endif