about summary refs log tree commit diff
path: root/src/librustc_codegen_llvm/back
diff options
context:
space:
mode:
authorAlex Crichton <alex@alexcrichton.com>2018-10-14 12:27:22 -0700
committerAlex Crichton <alex@alexcrichton.com>2018-10-19 02:35:00 -0700
commit3cc8f738d4247a9b475d8e074b621e602ac2b7be (patch)
treedba6d11f4bc2476e5e8bc3ed47d5986343651f06 /src/librustc_codegen_llvm/back
parentb1bdf04c97dd6631c627fe1a9d7fe2bb68e35dfd (diff)
downloadrust-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/back')
-rw-r--r--src/librustc_codegen_llvm/back/lto.rs12
-rw-r--r--src/librustc_codegen_llvm/back/write.rs34
2 files changed, 38 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.
         //