Improve support for NewPM

compiler/rustc_codegen_llvm/src/back/lto.rs

@@ -568,10 +568,11 @@ fn thin_lto(
 
 pub(crate) fn run_pass_manager(
     cgcx: &CodegenContext<LlvmCodegenBackend>,
+    diag_handler: &Handler,
     module: &ModuleCodegen<ModuleLlvm>,
     config: &ModuleConfig,
     thin: bool,
-) {
+) -> Result<(), FatalError> {
     let _timer = cgcx.prof.extra_verbose_generic_activity("LLVM_lto_optimize", &module.name[..]);
 
     // Now we have one massive module inside of llmod. Time to run the
@@ -584,15 +585,16 @@ pub(crate) fn run_pass_manager(
         if write::should_use_new_llvm_pass_manager(config) {
             let opt_stage = if thin { llvm::OptStage::ThinLTO } else { llvm::OptStage::FatLTO };
             let opt_level = config.opt_level.unwrap_or(config::OptLevel::No);
-            // See comment below for why this is necessary.
-            let opt_level = if let config::OptLevel::No = opt_level {
-                config::OptLevel::Less
-            } else {
-                opt_level
-            };
-            write::optimize_with_new_llvm_pass_manager(cgcx, module, config, opt_level, opt_stage);
+            write::optimize_with_new_llvm_pass_manager(
+                cgcx,
+                diag_handler,
+                module,
+                config,
+                opt_level,
+                opt_stage,
+            )?;
             debug!("lto done");
-            return;
+            return Ok(());
         }
 
         let pm = llvm::LLVMCreatePassManager();
@@ -603,26 +605,10 @@ pub(crate) fn run_pass_manager(
             llvm::LLVMRustAddPass(pm, pass.unwrap());
         }
 
-        // When optimizing for LTO we don't actually pass in `-O0`, but we force
-        // it to always happen at least with `-O1`.
-        //
-        // With ThinLTO we mess around a lot with symbol visibility in a way
-        // that will actually cause linking failures if we optimize at O0 which
-        // notable is lacking in dead code elimination. To ensure we at least
-        // get some optimizations and correctly link we forcibly switch to `-O1`
-        // to get dead code elimination.
-        //
-        // Note that in general this shouldn't matter too much as you typically
-        // only turn on ThinLTO when you're compiling with optimizations
-        // otherwise.
         let opt_level = config
             .opt_level
             .map(|x| to_llvm_opt_settings(x).0)
             .unwrap_or(llvm::CodeGenOptLevel::None);
-        let opt_level = match opt_level {
-            llvm::CodeGenOptLevel::None => llvm::CodeGenOptLevel::Less,
-            level => level,
-        };
         with_llvm_pmb(module.module_llvm.llmod(), config, opt_level, false, &mut |b| {
             if thin {
                 llvm::LLVMRustPassManagerBuilderPopulateThinLTOPassManager(b, pm);
@@ -650,6 +636,7 @@ pub(crate) fn run_pass_manager(
         llvm::LLVMDisposePassManager(pm);
     }
     debug!("lto done");
+    Ok(())
 }
 
 pub struct ModuleBuffer(&'static mut llvm::ModuleBuffer);
@@ -872,7 +859,7 @@ pub unsafe fn optimize_thin_module(
         {
             info!("running thin lto passes over {}", module.name);
             let config = cgcx.config(module.kind);
-            run_pass_manager(cgcx, &module, config, true);
+            run_pass_manager(cgcx, &diag_handler, &module, config, true)?;
             save_temp_bitcode(cgcx, &module, "thin-lto-after-pm");
         }
     }

compiler/rustc_codegen_llvm/src/back/write.rs

@@ -410,16 +410,17 @@ fn get_pgo_use_path(config: &ModuleConfig) -> Option<CString> {
 
 pub(crate) fn should_use_new_llvm_pass_manager(config: &ModuleConfig) -> bool {
     // The new pass manager is disabled by default.
-    config.new_llvm_pass_manager
+    config.new_llvm_pass_manager.unwrap_or(false)
 }
 
 pub(crate) unsafe fn optimize_with_new_llvm_pass_manager(
     cgcx: &CodegenContext<LlvmCodegenBackend>,
+    diag_handler: &Handler,
     module: &ModuleCodegen<ModuleLlvm>,
     config: &ModuleConfig,
     opt_level: config::OptLevel,
     opt_stage: llvm::OptStage,
-) {
+) -> Result<(), FatalError> {
     let unroll_loops =
         opt_level != config::OptLevel::Size && opt_level != config::OptLevel::SizeMin;
     let using_thin_buffers = opt_stage == llvm::OptStage::PreLinkThinLTO || config.bitcode_needed();
@@ -449,13 +450,12 @@ pub(crate) unsafe fn optimize_with_new_llvm_pass_manager(
         std::ptr::null_mut()
     };
 
+    let extra_passes = config.passes.join(",");
+
     // FIXME: NewPM doesn't provide a facility to pass custom InlineParams.
     // We would have to add upstream support for this first, before we can support
     // config.inline_threshold and our more aggressive default thresholds.
-    // FIXME: NewPM uses an different and more explicit way to textually represent
-    // pass pipelines. It would probably make sense to expose this, but it would
-    // require a different format than the current -C passes.
-    llvm::LLVMRustOptimizeWithNewPassManager(
+    let result = llvm::LLVMRustOptimizeWithNewPassManager(
         module.module_llvm.llmod(),
         &*module.module_llvm.tm,
         to_pass_builder_opt_level(opt_level),
@@ -472,10 +472,15 @@ pub(crate) unsafe fn optimize_with_new_llvm_pass_manager(
         sanitizer_options.as_ref(),
         pgo_gen_path.as_ref().map_or(std::ptr::null(), |s| s.as_ptr()),
         pgo_use_path.as_ref().map_or(std::ptr::null(), |s| s.as_ptr()),
+        config.instrument_coverage,
+        config.instrument_gcov,
         llvm_selfprofiler,
         selfprofile_before_pass_callback,
         selfprofile_after_pass_callback,
+        extra_passes.as_ptr().cast(),
+        extra_passes.len(),
     );
+    result.into_result().map_err(|()| llvm_err(diag_handler, "failed to run LLVM passes"))
 }
 
 // Unsafe due to LLVM calls.
@@ -484,7 +489,7 @@ pub(crate) unsafe fn optimize(
     diag_handler: &Handler,
     module: &ModuleCodegen<ModuleLlvm>,
     config: &ModuleConfig,
-) {
+) -> Result<(), FatalError> {
     let _timer = cgcx.prof.generic_activity_with_arg("LLVM_module_optimize", &module.name[..]);
 
     let llmod = module.module_llvm.llmod();
@@ -509,8 +514,14 @@ pub(crate) unsafe fn optimize(
                 _ if cgcx.opts.cg.linker_plugin_lto.enabled() => llvm::OptStage::PreLinkThinLTO,
                 _ => llvm::OptStage::PreLinkNoLTO,
             };
-            optimize_with_new_llvm_pass_manager(cgcx, module, config, opt_level, opt_stage);
-            return;
+            return optimize_with_new_llvm_pass_manager(
+                cgcx,
+                diag_handler,
+                module,
+                config,
+                opt_level,
+                opt_stage,
+            );
         }
 
         if cgcx.prof.llvm_recording_enabled() {
@@ -545,15 +556,6 @@ pub(crate) unsafe fn optimize(
                     llvm::LLVMRustAddPass(fpm, find_pass("lint").unwrap());
                     continue;
                 }
-                if pass_name == "insert-gcov-profiling" || pass_name == "instrprof" {
-                    // Instrumentation must be inserted before optimization,
-                    // otherwise LLVM may optimize some functions away which
-                    // breaks llvm-cov.
-                    //
-                    // This mirrors what Clang does in lib/CodeGen/BackendUtil.cpp.
-                    llvm::LLVMRustAddPass(mpm, find_pass(pass_name).unwrap());
-                    continue;
-                }
 
                 if let Some(pass) = find_pass(pass_name) {
                     extra_passes.push(pass);
@@ -566,6 +568,18 @@ pub(crate) unsafe fn optimize(
                 }
             }
 
+            // Instrumentation must be inserted before optimization,
+            // otherwise LLVM may optimize some functions away which
+            // breaks llvm-cov.
+            //
+            // This mirrors what Clang does in lib/CodeGen/BackendUtil.cpp.
+            if config.instrument_gcov {
+                llvm::LLVMRustAddPass(mpm, find_pass("insert-gcov-profiling").unwrap());
+            }
+            if config.instrument_coverage {
+                llvm::LLVMRustAddPass(mpm, find_pass("instrprof").unwrap());
+            }
+
             add_sanitizer_passes(config, &mut extra_passes);
 
             // Some options cause LLVM bitcode to be emitted, which uses ThinLTOBuffers, so we need
@@ -642,6 +656,7 @@ pub(crate) unsafe fn optimize(
         llvm::LLVMDisposePassManager(fpm);
         llvm::LLVMDisposePassManager(mpm);
     }
+    Ok(())
 }
 
 unsafe fn add_sanitizer_passes(config: &ModuleConfig, passes: &mut Vec<&'static mut llvm::Pass>) {

compiler/rustc_codegen_llvm/src/lib.rs

@@ -162,7 +162,7 @@ impl WriteBackendMethods for LlvmCodegenBackend {
         module: &ModuleCodegen<Self::Module>,
         config: &ModuleConfig,
     ) -> Result<(), FatalError> {
-        Ok(back::write::optimize(cgcx, diag_handler, module, config))
+        back::write::optimize(cgcx, diag_handler, module, config)
     }
     unsafe fn optimize_thin(
         cgcx: &CodegenContext<Self>,
@@ -189,8 +189,9 @@ impl WriteBackendMethods for LlvmCodegenBackend {
         module: &ModuleCodegen<Self::Module>,
         config: &ModuleConfig,
         thin: bool,
-    ) {
-        back::lto::run_pass_manager(cgcx, module, config, thin)
+    ) -> Result<(), FatalError> {
+        let diag_handler = cgcx.create_diag_handler();
+        back::lto::run_pass_manager(cgcx, &diag_handler, module, config, thin)
     }
 }
 

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

@@ -2203,10 +2203,14 @@ extern "C" {
         SanitizerOptions: Option<&SanitizerOptions>,
         PGOGenPath: *const c_char,
         PGOUsePath: *const c_char,
+        InstrumentCoverage: bool,
+        InstrumentGCOV: bool,
         llvm_selfprofiler: *mut c_void,
         begin_callback: SelfProfileBeforePassCallback,
         end_callback: SelfProfileAfterPassCallback,
-    );
+        ExtraPasses: *const c_char,
+        ExtraPassesLen: size_t,
+    ) -> LLVMRustResult;
     pub fn LLVMRustPrintModule(
         M: &'a Module,
         Output: *const c_char,

compiler/rustc_codegen_ssa/src/back/lto.rs

@@ -72,7 +72,7 @@ impl<B: WriteBackendMethods> LtoModuleCodegen<B> {
                 let module = module.take().unwrap();
                 {
                     let config = cgcx.config(module.kind);
-                    B::run_lto_pass_manager(cgcx, &module, config, false);
+                    B::run_lto_pass_manager(cgcx, &module, config, false)?;
                 }
                 Ok(module)
             }

compiler/rustc_codegen_ssa/src/back/write.rs

@@ -84,6 +84,8 @@ pub struct ModuleConfig {
 
     pub pgo_gen: SwitchWithOptPath,
     pub pgo_use: Option<PathBuf>,
+    pub instrument_coverage: bool,
+    pub instrument_gcov: bool,
 
     pub sanitizer: SanitizerSet,
     pub sanitizer_recover: SanitizerSet,
@@ -108,7 +110,7 @@ pub struct ModuleConfig {
     pub vectorize_slp: bool,
     pub merge_functions: bool,
     pub inline_threshold: Option<u32>,
-    pub new_llvm_pass_manager: bool,
+    pub new_llvm_pass_manager: Option<bool>,
     pub emit_lifetime_markers: bool,
 }
 
@@ -165,25 +167,7 @@ impl ModuleConfig {
         };
 
         ModuleConfig {
-            passes: if_regular!(
-                {
-                    let mut passes = sess.opts.cg.passes.clone();
-                    // compiler_builtins overrides the codegen-units settings,
-                    // which is incompatible with -Zprofile which requires that
-                    // only a single codegen unit is used per crate.
-                    if sess.opts.debugging_opts.profile && !is_compiler_builtins {
-                        passes.push("insert-gcov-profiling".to_owned());
-                    }
-
-                    // The rustc option `-Zinstrument_coverage` injects intrinsic calls to
-                    // `llvm.instrprof.increment()`, which requires the LLVM `instrprof` pass.
-                    if sess.instrument_coverage() {
-                        passes.push("instrprof".to_owned());
-                    }
-                    passes
-                },
-                vec![]
-            ),
+            passes: if_regular!(sess.opts.cg.passes.clone(), vec![]),
 
             opt_level: opt_level_and_size,
             opt_size: opt_level_and_size,
@@ -193,6 +177,14 @@ impl ModuleConfig {
                 SwitchWithOptPath::Disabled
             ),
             pgo_use: if_regular!(sess.opts.cg.profile_use.clone(), None),
+            instrument_coverage: if_regular!(sess.instrument_coverage(), false),
+            instrument_gcov: if_regular!(
+                // compiler_builtins overrides the codegen-units settings,
+                // which is incompatible with -Zprofile which requires that
+                // only a single codegen unit is used per crate.
+                sess.opts.debugging_opts.profile && !is_compiler_builtins,
+                false
+            ),
 
             sanitizer: if_regular!(sess.opts.debugging_opts.sanitizer, SanitizerSet::empty()),
             sanitizer_recover: if_regular!(

compiler/rustc_codegen_ssa/src/traits/write.rs

@@ -58,7 +58,7 @@ pub trait WriteBackendMethods: 'static + Sized + Clone {
         llmod: &ModuleCodegen<Self::Module>,
         config: &ModuleConfig,
         thin: bool,
-    );
+    ) -> Result<(), FatalError>;
 }
 
 pub trait ThinBufferMethods: Send + Sync {

compiler/rustc_interface/src/tests.rs

@@ -709,7 +709,7 @@ fn test_debugging_options_tracking_hash() {
     tracked!(mir_emit_retag, true);
     tracked!(mir_opt_level, Some(4));
     tracked!(mutable_noalias, Some(true));
-    tracked!(new_llvm_pass_manager, true);
+    tracked!(new_llvm_pass_manager, Some(true));
     tracked!(no_codegen, true);
     tracked!(no_generate_arange_section, true);
     tracked!(no_link, true);