diff options
| author | Alex Crichton <alex@alexcrichton.com> | 2018-10-14 12:27:22 -0700 |
|---|---|---|
| committer | Alex Crichton <alex@alexcrichton.com> | 2018-10-19 02:35:00 -0700 |
| commit | 3cc8f738d4247a9b475d8e074b621e602ac2b7be (patch) | |
| tree | dba6d11f4bc2476e5e8bc3ed47d5986343651f06 /src/librustc_codegen_llvm | |
| parent | b1bdf04c97dd6631c627fe1a9d7fe2bb68e35dfd (diff) | |
| download | rust-3cc8f738d4247a9b475d8e074b621e602ac2b7be.tar.gz rust-3cc8f738d4247a9b475d8e074b621e602ac2b7be.zip | |
rustc: Fix (again) simd vectors by-val in ABI
The issue of passing around SIMD types as values between functions has seen [quite a lot] of [discussion], and although we thought [we fixed it][quite a lot] it [wasn't]! This PR is a change to rustc to, again, try to fix this issue. The fundamental problem here remains the same, if a SIMD vector argument is passed by-value in LLVM's function type, then if the caller and callee disagree on target features a miscompile happens. We solve this by never passing SIMD vectors by-value, but LLVM will still thwart us with its argument promotion pass to promote by-ref SIMD arguments to by-val SIMD arguments. This commit is an attempt to thwart LLVM thwarting us. We, just before codegen, will take yet another look at the LLVM module and demote any by-value SIMD arguments we see. This is a very manual attempt by us to ensure the codegen for a module keeps working, and it unfortunately is likely producing suboptimal code, even in release mode. The saving grace for this, in theory, is that if SIMD types are passed by-value across a boundary in release mode it's pretty unlikely to be performance sensitive (as it's already doing a load/store, and otherwise perf-sensitive bits should be inlined). The implementation here is basically a big wad of C++. It was largely copied from LLVM's own argument promotion pass, only doing the reverse. In local testing this... Closes #50154 Closes #52636 Closes #54583 Closes #55059 [quite a lot]: https://github.com/rust-lang/rust/pull/47743 [discussion]: https://github.com/rust-lang/rust/issues/44367 [wasn't]: https://github.com/rust-lang/rust/issues/50154
Diffstat (limited to 'src/librustc_codegen_llvm')
| -rw-r--r-- | src/librustc_codegen_llvm/back/lto.rs | 12 | ||||
| -rw-r--r-- | src/librustc_codegen_llvm/back/write.rs | 34 | ||||
| -rw-r--r-- | src/librustc_codegen_llvm/llvm/ffi.rs | 2 |
3 files changed, 40 insertions, 8 deletions
diff --git a/src/librustc_codegen_llvm/back/lto.rs b/src/librustc_codegen_llvm/back/lto.rs index 3ac22f4eaef..f4afe521358 100644 --- a/src/librustc_codegen_llvm/back/lto.rs +++ b/src/librustc_codegen_llvm/back/lto.rs @@ -80,9 +80,7 @@ impl LtoModuleCodegen { let module = module.take().unwrap(); { let config = cgcx.config(module.kind); - let llmod = module.module_llvm.llmod(); - let tm = &*module.module_llvm.tm; - run_pass_manager(cgcx, tm, llmod, config, false); + run_pass_manager(cgcx, &module, config, false); timeline.record("fat-done"); } Ok(module) @@ -557,8 +555,7 @@ fn thin_lto(cgcx: &CodegenContext, } fn run_pass_manager(cgcx: &CodegenContext, - tm: &llvm::TargetMachine, - llmod: &llvm::Module, + module: &ModuleCodegen, config: &ModuleConfig, thin: bool) { // Now we have one massive module inside of llmod. Time to run the @@ -569,7 +566,8 @@ fn run_pass_manager(cgcx: &CodegenContext, debug!("running the pass manager"); unsafe { let pm = llvm::LLVMCreatePassManager(); - llvm::LLVMRustAddAnalysisPasses(tm, pm, llmod); + let llmod = module.module_llvm.llmod(); + llvm::LLVMRustAddAnalysisPasses(module.module_llvm.tm, pm, llmod); if config.verify_llvm_ir { let pass = llvm::LLVMRustFindAndCreatePass("verify\0".as_ptr() as *const _); @@ -864,7 +862,7 @@ impl ThinModule { // little differently. info!("running thin lto passes over {}", module.name); let config = cgcx.config(module.kind); - run_pass_manager(cgcx, module.module_llvm.tm, llmod, config, true); + run_pass_manager(cgcx, &module, config, true); cgcx.save_temp_bitcode(&module, "thin-lto-after-pm"); timeline.record("thin-done"); } diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 02ef690b942..f4a7afc1667 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -633,7 +633,7 @@ unsafe fn optimize(cgcx: &CodegenContext, None, &format!("llvm module passes [{}]", module_name.unwrap()), || { - llvm::LLVMRunPassManager(mpm, llmod) + llvm::LLVMRunPassManager(mpm, llmod); }); // Deallocate managers that we're now done with @@ -691,6 +691,38 @@ unsafe fn codegen(cgcx: &CodegenContext, create_msvc_imps(cgcx, llcx, llmod); } + // Ok now this one's a super interesting invocations. SIMD in rustc is + // difficult where we want some parts of the program to be able to use + // some SIMD features while other parts of the program don't. The real + // tough part is that we want this to actually work correctly! + // + // We go to great lengths to make sure this works, and one crucial + // aspect is that vector arguments (simd types) are never passed by + // value in the ABI of functions. It turns out, however, that LLVM will + // undo our "clever work" of passing vector types by reference. Its + // argument promotion pass will promote these by-ref arguments to + // by-val. That, however, introduces codegen errors! + // + // The upstream LLVM bug [1] has unfortunatey not really seen a lot of + // activity. The Rust bug [2], however, has seen quite a lot of reports + // of this in the wild. As a result, this is worked around locally here. + // We have a custom transformation, `LLVMRustDemoteSimdArguments`, which + // does the opposite of argument promotion by demoting any by-value SIMD + // arguments in function signatures to pointers intead of being + // by-value. + // + // This operates at the LLVM IR layer because LLVM is thwarting our + // codegen and this is the only chance we get to make sure it's correct + // before we hit codegen. + // + // Hopefully one day the upstream LLVM bug will be fixed and we'll no + // longer need this! + // + // [1]: https://bugs.llvm.org/show_bug.cgi?id=37358 + // [2]: https://github.com/rust-lang/rust/issues/50154 + llvm::LLVMRustDemoteSimdArguments(llmod); + cgcx.save_temp_bitcode(&module, "simd-demoted"); + // A codegen-specific pass manager is used to generate object // files for an LLVM module. // diff --git a/src/librustc_codegen_llvm/llvm/ffi.rs b/src/librustc_codegen_llvm/llvm/ffi.rs index c9f51efdc50..3286377ad32 100644 --- a/src/librustc_codegen_llvm/llvm/ffi.rs +++ b/src/librustc_codegen_llvm/llvm/ffi.rs @@ -1136,6 +1136,8 @@ extern "C" { /// Runs a pass manager on a module. pub fn LLVMRunPassManager(PM: &PassManager<'a>, M: &'a Module) -> Bool; + pub fn LLVMRustDemoteSimdArguments(M: &'a Module); + pub fn LLVMInitializePasses(); pub fn LLVMPassManagerBuilderCreate() -> &'static mut PassManagerBuilder; |
