Auto merge of #83894 - nikic:newpm, r=nagisa

Improve support for NewPM

This adds various missing bits of support for NewPM and allows us to successfully run stage 2 tests with NewPM enabled.

This does not yet enable NewPM by default, as there are still known issue on LLVM 12 (such as a weak fat LTO pipeline). The plan is to make the switch after we update to LLVM 13.

10 files changed, 103 insertions, 74 deletions

diff --git a/compiler/rustc_codegen_llvm/src/back/lto.rs b/compiler/rustc_codegen_llvm/src/back/lto.rs
index 4226ed7d99b..f612785e5a4 100644
--- a/compiler/rustc_codegen_llvm/src/back/lto.rs
+++ b/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");
         }
     }
diff --git a/compiler/rustc_codegen_llvm/src/back/write.rs b/compiler/rustc_codegen_llvm/src/back/write.rs
index b628ae3ae3a..4219797c5cd 100644
--- a/compiler/rustc_codegen_llvm/src/back/write.rs
+++ b/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>) {
diff --git a/compiler/rustc_codegen_llvm/src/lib.rs b/compiler/rustc_codegen_llvm/src/lib.rs
index 5ca4b226c38..bc35aa72965 100644
--- a/compiler/rustc_codegen_llvm/src/lib.rs
+++ b/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)
     }
 }
 
diff --git a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs b/compiler/rustc_codegen_llvm/src/llvm/ffi.rs
index 32b1526f6e4..32fdde9b42e 100644
--- a/compiler/rustc_codegen_llvm/src/llvm/ffi.rs
+++ b/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,
diff --git a/compiler/rustc_codegen_ssa/src/back/lto.rs b/compiler/rustc_codegen_ssa/src/back/lto.rs
index 0ff05229466..d6ae689f254 100644
--- a/compiler/rustc_codegen_ssa/src/back/lto.rs
+++ b/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)
             }
diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs
index c8688faa80b..0dfb007a247 100644
--- a/compiler/rustc_codegen_ssa/src/back/write.rs
+++ b/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!(
diff --git a/compiler/rustc_codegen_ssa/src/traits/write.rs b/compiler/rustc_codegen_ssa/src/traits/write.rs
index 264e7c2aa92..93fbee2b49b 100644
--- a/compiler/rustc_codegen_ssa/src/traits/write.rs
+++ b/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 {
diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs
index 4d87bbead41..fd13cb3d59a 100644
--- a/compiler/rustc_interface/src/tests.rs
+++ b/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);
diff --git a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
index 65a988629c3..99ce13a6ed5 100644
--- a/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
+++ b/compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
@@ -32,6 +32,8 @@
 #include "llvm/Transforms/Instrumentation.h"
 #include "llvm/Transforms/Instrumentation/AddressSanitizer.h"
 #include "llvm/Support/TimeProfiler.h"
+#include "llvm/Transforms/Instrumentation/GCOVProfiler.h"
+#include "llvm/Transforms/Instrumentation/InstrProfiling.h"
 #include "llvm/Transforms/Instrumentation/ThreadSanitizer.h"
 #include "llvm/Transforms/Instrumentation/MemorySanitizer.h"
 #include "llvm/Transforms/Instrumentation/HWAddressSanitizer.h"
@@ -734,7 +736,7 @@ struct LLVMRustSanitizerOptions {
   bool SanitizeHWAddressRecover;
 };
 
-extern "C" void
+extern "C" LLVMRustResult
 LLVMRustOptimizeWithNewPassManager(
     LLVMModuleRef ModuleRef,
     LLVMTargetMachineRef TMRef,
@@ -745,9 +747,11 @@ LLVMRustOptimizeWithNewPassManager(
     bool DisableSimplifyLibCalls, bool EmitLifetimeMarkers,
     LLVMRustSanitizerOptions *SanitizerOptions,
     const char *PGOGenPath, const char *PGOUsePath,
+    bool InstrumentCoverage, bool InstrumentGCOV,
     void* LlvmSelfProfiler,
     LLVMRustSelfProfileBeforePassCallback BeforePassCallback,
-    LLVMRustSelfProfileAfterPassCallback AfterPassCallback) {
+    LLVMRustSelfProfileAfterPassCallback AfterPassCallback,
+    const char *ExtraPasses, size_t ExtraPassesLen) {
   Module *TheModule = unwrap(ModuleRef);
   TargetMachine *TM = unwrap(TMRef);
   PassBuilder::OptimizationLevel OptLevel = fromRust(OptLevelRust);
@@ -834,6 +838,23 @@ LLVMRustOptimizeWithNewPassManager(
     );
   }
 
+  if (InstrumentGCOV) {
+    PipelineStartEPCallbacks.push_back(
+      [](ModulePassManager &MPM, PassBuilder::OptimizationLevel Level) {
+        MPM.addPass(GCOVProfilerPass(GCOVOptions::getDefault()));
+      }
+    );
+  }
+
+  if (InstrumentCoverage) {
+    PipelineStartEPCallbacks.push_back(
+      [](ModulePassManager &MPM, PassBuilder::OptimizationLevel Level) {
+        InstrProfOptions Options;
+        MPM.addPass(InstrProfiling(Options, false));
+      }
+    );
+  }
+
   if (SanitizerOptions) {
     if (SanitizerOptions->SanitizeMemory) {
       MemorySanitizerOptions Options(
@@ -1042,6 +1063,14 @@ LLVMRustOptimizeWithNewPassManager(
     }
   }
 
+  if (ExtraPassesLen) {
+    if (auto Err = PB.parsePassPipeline(MPM, StringRef(ExtraPasses, ExtraPassesLen))) {
+      std::string ErrMsg = toString(std::move(Err));
+      LLVMRustSetLastError(ErrMsg.c_str());
+      return LLVMRustResult::Failure;
+    }
+  }
+
   if (NeedThinLTOBufferPasses) {
     MPM.addPass(CanonicalizeAliasesPass());
     MPM.addPass(NameAnonGlobalPass());
@@ -1052,6 +1081,7 @@ LLVMRustOptimizeWithNewPassManager(
     UpgradeCallsToIntrinsic(&*I++); // must be post-increment, as we remove
 
   MPM.run(*TheModule, MAM);
+  return LLVMRustResult::Success;
 }
 
 // Callback to demangle function name
diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs
index a59f0462c73..da6e2af2760 100644
--- a/compiler/rustc_session/src/options.rs
+++ b/compiler/rustc_session/src/options.rs
@@ -1112,7 +1112,7 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
         "MIR optimization level (0-4; default: 1 in non optimized builds and 2 in optimized builds)"),
     mutable_noalias: Option<bool> = (None, parse_opt_bool, [TRACKED],
         "emit noalias metadata for mutable references (default: yes for LLVM >= 12, otherwise no)"),
-    new_llvm_pass_manager: bool = (false, parse_bool, [TRACKED],
+    new_llvm_pass_manager: Option<bool> = (None, parse_opt_bool, [TRACKED],
         "use new LLVM pass manager (default: no)"),
     nll_facts: bool = (false, parse_bool, [UNTRACKED],
         "dump facts from NLL analysis into side files (default: no)"),