Don't add extra passes into the function pass manager

Exception for specific cases like linting, additional passes should
be going into the module pass manager (even if they are function
passes). The separate function pass manager is only used for very
early optimization passes.

Rather than apparending passes to the MPM, use the OptimizerLast
and EnabledOnOptLevel0 pass manager builder extension hooks, which
allow adding passes directly before finalization (alias
canonicalization and name-anon-globals).

The main effect and purpose of this change is to add sanitizer
passes at the end of the pipeline, which is where they belong.
In LLVM 9 the address sanitizer can't be used as a pass in the
early function pass manager, because it has a dependence on a
module-level analysis pass.

Author: nikic

src/librustc_codegen_llvm/back/write.rs

@@ -329,33 +329,55 @@ pub(crate) unsafe fn optimize(cgcx: &CodegenContext<LlvmCodegenBackend>,
         let mpm = llvm::LLVMCreatePassManager();
 
         {
-            // If we're verifying or linting, add them to the function pass
-            // manager.
-            let addpass = |pass_name: &str| {
+            let find_pass = |pass_name: &str| {
                 let pass_name = SmallCStr::new(pass_name);
-                let pass = match llvm::LLVMRustFindAndCreatePass(pass_name.as_ptr()) {
-                    Some(pass) => pass,
-                    None => return false,
-                };
-                let pass_manager = match llvm::LLVMRustPassKind(pass) {
-                    llvm::PassKind::Function => &*fpm,
-                    llvm::PassKind::Module => &*mpm,
-                    llvm::PassKind::Other => {
-                        diag_handler.err("Encountered LLVM pass kind we can't handle");
-                        return true
-                    },
-                };
-                llvm::LLVMRustAddPass(pass_manager, pass);
-                true
+                llvm::LLVMRustFindAndCreatePass(pass_name.as_ptr())
             };
 
-            if config.verify_llvm_ir { assert!(addpass("verify")); }
+            if config.verify_llvm_ir {
+                // Verification should run as the very first pass.
+                llvm::LLVMRustAddPass(fpm, find_pass("verify").unwrap());
+            }
+
+            let mut extra_passes = Vec::new();
+            let mut have_name_anon_globals_pass = false;
+
+            for pass_name in &config.passes {
+                if pass_name == "lint" {
+                    // Linting should also be performed early, directly on the generated IR.
+                    llvm::LLVMRustAddPass(fpm, find_pass("lint").unwrap());
+                    continue;
+                }
+
+                if let Some(pass) = find_pass(pass_name) {
+                    extra_passes.push(pass);
+                } else {
+                    diag_handler.warn(&format!("unknown pass `{}`, ignoring", pass_name));
+                }
+
+                if pass_name == "name-anon-globals" {
+                    have_name_anon_globals_pass = true;
+                }
+            }
+
+            for pass_name in &cgcx.plugin_passes {
+                if let Some(pass) = find_pass(pass_name) {
+                    extra_passes.push(pass);
+                } else {
+                    diag_handler.err(&format!("a plugin asked for LLVM pass \
+                                               `{}` but LLVM does not \
+                                               recognize it", pass_name));
+                }
+
+                if pass_name == "name-anon-globals" {
+                    have_name_anon_globals_pass = true;
+                }
+            }
 
             // Some options cause LLVM bitcode to be emitted, which uses ThinLTOBuffers, so we need
             // to make sure we run LLVM's NameAnonGlobals pass when emitting bitcode; otherwise
             // we'll get errors in LLVM.
             let using_thin_buffers = config.bitcode_needed();
-            let mut have_name_anon_globals_pass = false;
             if !config.no_prepopulate_passes {
                 llvm::LLVMRustAddAnalysisPasses(tm, fpm, llmod);
                 llvm::LLVMRustAddAnalysisPasses(tm, mpm, llmod);
@@ -364,34 +386,22 @@ pub(crate) unsafe fn optimize(cgcx: &CodegenContext<LlvmCodegenBackend>,
                 let prepare_for_thin_lto = cgcx.lto == Lto::Thin || cgcx.lto == Lto::ThinLocal ||
                     (cgcx.lto != Lto::Fat && cgcx.opts.cg.linker_plugin_lto.enabled());
                 with_llvm_pmb(llmod, &config, opt_level, prepare_for_thin_lto, &mut |b| {
+                    llvm::LLVMRustAddLastExtensionPasses(
+                        b, extra_passes.as_ptr(), extra_passes.len() as size_t);
                     llvm::LLVMPassManagerBuilderPopulateFunctionPassManager(b, fpm);
                     llvm::LLVMPassManagerBuilderPopulateModulePassManager(b, mpm);
                 });
 
                 have_name_anon_globals_pass = have_name_anon_globals_pass || prepare_for_thin_lto;
                 if using_thin_buffers && !prepare_for_thin_lto {
-                    assert!(addpass("name-anon-globals"));
-                    have_name_anon_globals_pass = true;
-                }
-            }
-
-            for pass in &config.passes {
-                if !addpass(pass) {
-                    diag_handler.warn(&format!("unknown pass `{}`, ignoring", pass));
-                }
-                if pass == "name-anon-globals" {
+                    llvm::LLVMRustAddPass(mpm, find_pass("name-anon-globals").unwrap());
                     have_name_anon_globals_pass = true;
                 }
-            }
-
-            for pass in &cgcx.plugin_passes {
-                if !addpass(pass) {
-                    diag_handler.err(&format!("a plugin asked for LLVM pass \
-                                               `{}` but LLVM does not \
-                                               recognize it", pass));
-                }
-                if pass == "name-anon-globals" {
-                    have_name_anon_globals_pass = true;
+            } else {
+                // If we don't use the standard pipeline, directly populate the MPM
+                // with the extra passes.
+                for pass in extra_passes {
+                    llvm::LLVMRustAddPass(mpm, pass);
                 }
             }
 

src/librustc_codegen_llvm/llvm/ffi.rs

@@ -1668,6 +1668,9 @@ extern "C" {
     pub fn LLVMRustPassKind(Pass: &Pass) -> PassKind;
     pub fn LLVMRustFindAndCreatePass(Pass: *const c_char) -> Option<&'static mut Pass>;
     pub fn LLVMRustAddPass(PM: &PassManager<'_>, Pass: &'static mut Pass);
+    pub fn LLVMRustAddLastExtensionPasses(PMB: &PassManagerBuilder,
+                                          Passes: *const &'static mut Pass,
+                                          NumPasses: size_t);
 
     pub fn LLVMRustHasFeature(T: &TargetMachine, s: *const c_char) -> bool;
 

src/rustllvm/PassWrapper.cpp

@@ -99,6 +99,23 @@ void LLVMRustPassManagerBuilderPopulateThinLTOPassManager(
   unwrap(PMBR)->populateThinLTOPassManager(*unwrap(PMR));
 }
 
+extern "C"
+void LLVMRustAddLastExtensionPasses(
+    LLVMPassManagerBuilderRef PMBR, LLVMPassRef *Passes, size_t NumPasses) {
+  auto AddExtensionPasses = [Passes, NumPasses](
+      const PassManagerBuilder &Builder, PassManagerBase &PM) {
+    for (size_t I = 0; I < NumPasses; I++) {
+      PM.add(unwrap(Passes[I]));
+    }
+  };
+  // Add the passes to both of the pre-finalization extension points,
+  // so they are run for optimized and non-optimized builds.
+  unwrap(PMBR)->addExtension(PassManagerBuilder::EP_OptimizerLast,
+                             AddExtensionPasses);
+  unwrap(PMBR)->addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
+                             AddExtensionPasses);
+}
+
 #ifdef LLVM_COMPONENT_X86
 #define SUBTARGET_X86 SUBTARGET(X86)
 #else