From c54ce496635e0e013e14aa4be5d86c1247f9319a Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 19 Mar 2020 17:00:41 +1100 Subject: [PATCH 1/7] Remove unused `ModuleConfig::emit_lto_bc` field. --- src/librustc_codegen_ssa/back/write.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index dbc2ef6f2b0..87764593fda 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -74,7 +74,6 @@ pub struct ModuleConfig { pub emit_no_opt_bc: bool, pub emit_bc: bool, pub emit_bc_compressed: bool, - pub emit_lto_bc: bool, pub emit_ir: bool, pub emit_asm: bool, pub emit_obj: bool, @@ -116,7 +115,6 @@ impl ModuleConfig { emit_pre_lto_bc: false, emit_bc: false, emit_bc_compressed: false, - emit_lto_bc: false, emit_ir: false, emit_asm: false, emit_obj: false, @@ -381,7 +379,6 @@ pub fn start_async_codegen( modules_config.emit_no_opt_bc = true; modules_config.emit_pre_lto_bc = true; modules_config.emit_bc = true; - modules_config.emit_lto_bc = true; metadata_config.emit_bc = true; allocator_config.emit_bc = true; } From d156bf658fd2478ee83d8e86e8c7c46093b74ec8 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 20 Mar 2020 12:09:24 +1100 Subject: [PATCH 2/7] Remove some local variables. I find the code easier to read if the values in `config` are all used directly, rather than a mix of `config` values and local variables. It will also faciliate some of the following commits. Also, use `config.bitcode_needed()` in one place. --- src/librustc_codegen_llvm/back/write.rs | 32 ++++++++++--------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 0c243128104..9ffccad4b93 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -634,30 +634,24 @@ pub(crate) unsafe fn codegen( f(cpm) } - // If we don't have the integrated assembler, then we need to emit asm - // from LLVM and use `gcc` to create the object file. - let asm_to_obj = config.emit_obj && config.no_integrated_as; - - // Change what we write and cleanup based on whether obj files are - // just llvm bitcode. In that case write bitcode, and possibly - // delete the bitcode if it wasn't requested. Don't generate the - // machine code, instead copy the .o file from the .bc - let write_bc = config.emit_bc || config.obj_is_bitcode; - let rm_bc = !config.emit_bc && config.obj_is_bitcode; - let write_obj = config.emit_obj && !config.obj_is_bitcode && !asm_to_obj; - let copy_bc_to_obj = config.emit_obj && config.obj_is_bitcode; + // Two things to note: + // - If object files are just LLVM bitcode we write bitcode, copy it to + // the .o file, and delete the bitcode if it wasn't otherwise + // requested. + // - If we don't have the integrated assembler then we need to emit + // asm from LLVM and use `gcc` to create the object file. let bc_out = cgcx.output_filenames.temp_path(OutputType::Bitcode, module_name); let obj_out = cgcx.output_filenames.temp_path(OutputType::Object, module_name); - if write_bc || config.emit_bc_compressed || config.embed_bitcode { + if config.bitcode_needed() { let _timer = cgcx .prof .generic_activity_with_arg("LLVM_module_codegen_make_bitcode", &module.name[..]); let thin = ThinBuffer::new(llmod); let data = thin.data(); - if write_bc { + if config.emit_bc || config.obj_is_bitcode { let _timer = cgcx.prof.generic_activity_with_arg( "LLVM_module_codegen_emit_bitcode", &module.name[..], @@ -740,7 +734,7 @@ pub(crate) unsafe fn codegen( })?; } - if config.emit_asm || asm_to_obj { + if config.emit_asm || (config.emit_obj && config.no_integrated_as) { let _timer = cgcx .prof .generic_activity_with_arg("LLVM_module_codegen_emit_asm", &module.name[..]); @@ -762,7 +756,7 @@ pub(crate) unsafe fn codegen( })?; } - if write_obj { + if config.emit_obj && !config.obj_is_bitcode && !config.no_integrated_as { let _timer = cgcx .prof .generic_activity_with_arg("LLVM_module_codegen_emit_obj", &module.name[..]); @@ -776,7 +770,7 @@ pub(crate) unsafe fn codegen( llvm::FileType::ObjectFile, ) })?; - } else if asm_to_obj { + } else if config.emit_obj && config.no_integrated_as { let _timer = cgcx .prof .generic_activity_with_arg("LLVM_module_codegen_asm_to_obj", &module.name[..]); @@ -789,14 +783,14 @@ pub(crate) unsafe fn codegen( } } - if copy_bc_to_obj { + if config.emit_obj && config.obj_is_bitcode { debug!("copying bitcode {:?} to obj {:?}", bc_out, obj_out); if let Err(e) = link_or_copy(&bc_out, &obj_out) { diag_handler.err(&format!("failed to copy bitcode to object file: {}", e)); } } - if rm_bc { + if !config.emit_bc && config.obj_is_bitcode { debug!("removing_bitcode {:?}", bc_out); if let Err(e) = fs::remove_file(&bc_out) { diag_handler.err(&format!("failed to remove bitcode: {}", e)); From 47c8f3f56babf7830c481cb679f81466c351db7b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 20 Mar 2020 12:34:39 +1100 Subject: [PATCH 3/7] Combine `ModuleConfig::embed_bitcode{,_marker}`. Because the `(true, true)` combination isn't valid. --- src/librustc_codegen_llvm/back/write.rs | 6 ++--- src/librustc_codegen_ssa/back/write.rs | 36 +++++++++++++++---------- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 9ffccad4b93..828c4a4bdf4 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -18,7 +18,7 @@ use rustc::bug; use rustc::session::config::{self, Lto, OutputType, Passes, Sanitizer, SwitchWithOptPath}; use rustc::session::Session; use rustc::ty::TyCtxt; -use rustc_codegen_ssa::back::write::{run_assembler, CodegenContext, ModuleConfig}; +use rustc_codegen_ssa::back::write::{run_assembler, CodegenContext, EmbedBitcode, ModuleConfig}; use rustc_codegen_ssa::traits::*; use rustc_codegen_ssa::{CompiledModule, ModuleCodegen, RLIB_BYTECODE_EXTENSION}; use rustc_data_structures::small_c_str::SmallCStr; @@ -662,7 +662,7 @@ pub(crate) unsafe fn codegen( } } - if config.embed_bitcode { + if config.embed_bitcode == EmbedBitcode::Full { let _timer = cgcx.prof.generic_activity_with_arg( "LLVM_module_codegen_embed_bitcode", &module.name[..], @@ -682,7 +682,7 @@ pub(crate) unsafe fn codegen( diag_handler.err(&msg); } } - } else if config.embed_bitcode_marker { + } else if config.embed_bitcode == EmbedBitcode::Marker { embed_bitcode(cgcx, llcx, llmod, None); } diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index 87764593fda..54d30487c79 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -51,6 +51,14 @@ use std::thread; const PRE_LTO_BC_EXT: &str = "pre-lto.bc"; +/// The kind of bitcode to embed in object files. +#[derive(PartialEq)] +pub enum EmbedBitcode { + None, + Marker, + Full, +} + /// Module-specific configuration for `optimize_and_codegen`. pub struct ModuleConfig { /// Names of additional optimization passes to run. @@ -93,8 +101,7 @@ pub struct ModuleConfig { // emscripten's ecc compiler, when used as the linker. pub obj_is_bitcode: bool, pub no_integrated_as: bool, - pub embed_bitcode: bool, - pub embed_bitcode_marker: bool, + pub embed_bitcode: EmbedBitcode, } impl ModuleConfig { @@ -119,8 +126,7 @@ impl ModuleConfig { emit_asm: false, emit_obj: false, obj_is_bitcode: false, - embed_bitcode: false, - embed_bitcode_marker: false, + embed_bitcode: EmbedBitcode::None, no_integrated_as: false, verify_llvm_ir: false, @@ -143,16 +149,15 @@ impl ModuleConfig { self.new_llvm_pass_manager = sess.opts.debugging_opts.new_llvm_pass_manager; self.obj_is_bitcode = sess.target.target.options.obj_is_bitcode || sess.opts.cg.linker_plugin_lto.enabled(); - let embed_bitcode = - sess.target.target.options.embed_bitcode || sess.opts.debugging_opts.embed_bitcode; - if embed_bitcode { - match sess.opts.optimize { - config::OptLevel::No | config::OptLevel::Less => { - self.embed_bitcode_marker = embed_bitcode; + self.embed_bitcode = + if sess.target.target.options.embed_bitcode || sess.opts.debugging_opts.embed_bitcode { + match sess.opts.optimize { + config::OptLevel::No | config::OptLevel::Less => EmbedBitcode::Marker, + _ => EmbedBitcode::Full, } - _ => self.embed_bitcode = embed_bitcode, - } - } + } else { + EmbedBitcode::None + }; // Copy what clang does by turning on loop vectorization at O2 and // slp vectorization at O3. Otherwise configure other optimization aspects @@ -188,7 +193,10 @@ impl ModuleConfig { } pub fn bitcode_needed(&self) -> bool { - self.emit_bc || self.obj_is_bitcode || self.emit_bc_compressed || self.embed_bitcode + self.emit_bc + || self.obj_is_bitcode + || self.emit_bc_compressed + || self.embed_bitcode == EmbedBitcode::Full } } From e4b36baf54f0799c744b3f30f5be425ef050ce8c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 20 Mar 2020 12:46:27 +1100 Subject: [PATCH 4/7] Remove an unnecessary block scope. --- src/librustc_codegen_llvm/back/write.rs | 154 +++++++++++------------- 1 file changed, 72 insertions(+), 82 deletions(-) diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 828c4a4bdf4..2826de1e490 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -686,100 +686,90 @@ pub(crate) unsafe fn codegen( embed_bitcode(cgcx, llcx, llmod, None); } - { - if config.emit_ir { - let _timer = cgcx - .prof - .generic_activity_with_arg("LLVM_module_codegen_emit_ir", &module.name[..]); - let out = cgcx.output_filenames.temp_path(OutputType::LlvmAssembly, module_name); - let out_c = path_to_c_string(&out); + if config.emit_ir { + let _timer = cgcx + .prof + .generic_activity_with_arg("LLVM_module_codegen_emit_ir", &module.name[..]); + let out = cgcx.output_filenames.temp_path(OutputType::LlvmAssembly, module_name); + let out_c = path_to_c_string(&out); - extern "C" fn demangle_callback( - input_ptr: *const c_char, - input_len: size_t, - output_ptr: *mut c_char, - output_len: size_t, - ) -> size_t { - let input = unsafe { - slice::from_raw_parts(input_ptr as *const u8, input_len as usize) - }; + extern "C" fn demangle_callback( + input_ptr: *const c_char, + input_len: size_t, + output_ptr: *mut c_char, + output_len: size_t, + ) -> size_t { + let input = + unsafe { slice::from_raw_parts(input_ptr as *const u8, input_len as usize) }; - let input = match str::from_utf8(input) { - Ok(s) => s, - Err(_) => return 0, - }; + let input = match str::from_utf8(input) { + Ok(s) => s, + Err(_) => return 0, + }; - let output = unsafe { - slice::from_raw_parts_mut(output_ptr as *mut u8, output_len as usize) - }; - let mut cursor = io::Cursor::new(output); + let output = unsafe { + slice::from_raw_parts_mut(output_ptr as *mut u8, output_len as usize) + }; + let mut cursor = io::Cursor::new(output); - let demangled = match rustc_demangle::try_demangle(input) { - Ok(d) => d, - Err(_) => return 0, - }; + let demangled = match rustc_demangle::try_demangle(input) { + Ok(d) => d, + Err(_) => return 0, + }; - if write!(cursor, "{:#}", demangled).is_err() { - // Possible only if provided buffer is not big enough - return 0; - } - - cursor.position() as size_t + if write!(cursor, "{:#}", demangled).is_err() { + // Possible only if provided buffer is not big enough + return 0; } - let result = llvm::LLVMRustPrintModule(llmod, out_c.as_ptr(), demangle_callback); - result.into_result().map_err(|()| { - let msg = format!("failed to write LLVM IR to {}", out.display()); - llvm_err(diag_handler, &msg) - })?; + cursor.position() as size_t } - if config.emit_asm || (config.emit_obj && config.no_integrated_as) { - let _timer = cgcx - .prof - .generic_activity_with_arg("LLVM_module_codegen_emit_asm", &module.name[..]); - let path = cgcx.output_filenames.temp_path(OutputType::Assembly, module_name); + let result = llvm::LLVMRustPrintModule(llmod, out_c.as_ptr(), demangle_callback); + result.into_result().map_err(|()| { + let msg = format!("failed to write LLVM IR to {}", out.display()); + llvm_err(diag_handler, &msg) + })?; + } - // We can't use the same module for asm and binary output, because that triggers - // various errors like invalid IR or broken binaries, so we might have to clone the - // module to produce the asm output - let llmod = if config.emit_obj { llvm::LLVMCloneModule(llmod) } else { llmod }; - with_codegen(tm, llmod, config.no_builtins, |cpm| { - write_output_file( - diag_handler, - tm, - cpm, - llmod, - &path, - llvm::FileType::AssemblyFile, - ) - })?; - } + if config.emit_asm || (config.emit_obj && config.no_integrated_as) { + let _timer = cgcx + .prof + .generic_activity_with_arg("LLVM_module_codegen_emit_asm", &module.name[..]); + let path = cgcx.output_filenames.temp_path(OutputType::Assembly, module_name); - if config.emit_obj && !config.obj_is_bitcode && !config.no_integrated_as { - let _timer = cgcx - .prof - .generic_activity_with_arg("LLVM_module_codegen_emit_obj", &module.name[..]); - with_codegen(tm, llmod, config.no_builtins, |cpm| { - write_output_file( - diag_handler, - tm, - cpm, - llmod, - &obj_out, - llvm::FileType::ObjectFile, - ) - })?; - } else if config.emit_obj && config.no_integrated_as { - let _timer = cgcx - .prof - .generic_activity_with_arg("LLVM_module_codegen_asm_to_obj", &module.name[..]); - let assembly = cgcx.output_filenames.temp_path(OutputType::Assembly, module_name); - run_assembler(cgcx, diag_handler, &assembly, &obj_out); + // We can't use the same module for asm and binary output, because that triggers + // various errors like invalid IR or broken binaries, so we might have to clone the + // module to produce the asm output + let llmod = if config.emit_obj { llvm::LLVMCloneModule(llmod) } else { llmod }; + with_codegen(tm, llmod, config.no_builtins, |cpm| { + write_output_file(diag_handler, tm, cpm, llmod, &path, llvm::FileType::AssemblyFile) + })?; + } - if !config.emit_asm && !cgcx.save_temps { - drop(fs::remove_file(&assembly)); - } + if config.emit_obj && !config.obj_is_bitcode && !config.no_integrated_as { + let _timer = cgcx + .prof + .generic_activity_with_arg("LLVM_module_codegen_emit_obj", &module.name[..]); + with_codegen(tm, llmod, config.no_builtins, |cpm| { + write_output_file( + diag_handler, + tm, + cpm, + llmod, + &obj_out, + llvm::FileType::ObjectFile, + ) + })?; + } else if config.emit_obj && config.no_integrated_as { + let _timer = cgcx + .prof + .generic_activity_with_arg("LLVM_module_codegen_asm_to_obj", &module.name[..]); + let assembly = cgcx.output_filenames.temp_path(OutputType::Assembly, module_name); + run_assembler(cgcx, diag_handler, &assembly, &obj_out); + + if !config.emit_asm && !cgcx.save_temps { + drop(fs::remove_file(&assembly)); } } From f8261b496d5c94bd8d6ad5dcce0cb5cc2e0e3fe5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Sat, 21 Mar 2020 09:15:07 +1100 Subject: [PATCH 5/7] Factor out a repeated `config.obj_is_bitcode` test. --- src/librustc_codegen_llvm/back/write.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 2826de1e490..1df8ee6746d 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -773,17 +773,19 @@ pub(crate) unsafe fn codegen( } } - if config.emit_obj && config.obj_is_bitcode { - debug!("copying bitcode {:?} to obj {:?}", bc_out, obj_out); - if let Err(e) = link_or_copy(&bc_out, &obj_out) { - diag_handler.err(&format!("failed to copy bitcode to object file: {}", e)); + if config.obj_is_bitcode { + if config.emit_obj { + debug!("copying bitcode {:?} to obj {:?}", bc_out, obj_out); + if let Err(e) = link_or_copy(&bc_out, &obj_out) { + diag_handler.err(&format!("failed to copy bitcode to object file: {}", e)); + } } - } - if !config.emit_bc && config.obj_is_bitcode { - debug!("removing_bitcode {:?}", bc_out); - if let Err(e) = fs::remove_file(&bc_out) { - diag_handler.err(&format!("failed to remove bitcode: {}", e)); + if !config.emit_bc { + debug!("removing_bitcode {:?}", bc_out); + if let Err(e) = fs::remove_file(&bc_out) { + diag_handler.err(&format!("failed to remove bitcode: {}", e)); + } } } From a147cd070e4b8824aafc7fa9f20d6fdf34eb3bf9 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Sat, 21 Mar 2020 09:19:21 +1100 Subject: [PATCH 6/7] Introduce a local variable `config_emit_normal_obj`. This adds a missing `!config.obj_is_bitcode` condition to two places that should have it. As a result, when `obj_is_bitcode` and `no_integrated_as` are both true, the compiler will no longer unnecessarily emit asm, convert it to an object file, and then overwrite that object file with bitcode. --- src/librustc_codegen_llvm/back/write.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index 1df8ee6746d..d2d3eb59796 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -732,7 +732,9 @@ pub(crate) unsafe fn codegen( })?; } - if config.emit_asm || (config.emit_obj && config.no_integrated_as) { + let config_emit_normal_obj = config.emit_obj && !config.obj_is_bitcode; + + if config.emit_asm || (config_emit_normal_obj && config.no_integrated_as) { let _timer = cgcx .prof .generic_activity_with_arg("LLVM_module_codegen_emit_asm", &module.name[..]); @@ -747,7 +749,7 @@ pub(crate) unsafe fn codegen( })?; } - if config.emit_obj && !config.obj_is_bitcode && !config.no_integrated_as { + if config_emit_normal_obj && !config.no_integrated_as { let _timer = cgcx .prof .generic_activity_with_arg("LLVM_module_codegen_emit_obj", &module.name[..]); @@ -761,7 +763,7 @@ pub(crate) unsafe fn codegen( llvm::FileType::ObjectFile, ) })?; - } else if config.emit_obj && config.no_integrated_as { + } else if config_emit_normal_obj && config.no_integrated_as { let _timer = cgcx .prof .generic_activity_with_arg("LLVM_module_codegen_asm_to_obj", &module.name[..]); From ca0f9975199b5fbe177dc61d493d411f67c66880 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Sat, 21 Mar 2020 09:25:20 +1100 Subject: [PATCH 7/7] Factor out a repeated `config.no_integrated_as` test. --- src/librustc_codegen_llvm/back/write.rs | 46 +++++++++++++------------ 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/librustc_codegen_llvm/back/write.rs b/src/librustc_codegen_llvm/back/write.rs index d2d3eb59796..62ac2381e8c 100644 --- a/src/librustc_codegen_llvm/back/write.rs +++ b/src/librustc_codegen_llvm/back/write.rs @@ -749,29 +749,31 @@ pub(crate) unsafe fn codegen( })?; } - if config_emit_normal_obj && !config.no_integrated_as { - let _timer = cgcx - .prof - .generic_activity_with_arg("LLVM_module_codegen_emit_obj", &module.name[..]); - with_codegen(tm, llmod, config.no_builtins, |cpm| { - write_output_file( - diag_handler, - tm, - cpm, - llmod, - &obj_out, - llvm::FileType::ObjectFile, - ) - })?; - } else if config_emit_normal_obj && config.no_integrated_as { - let _timer = cgcx - .prof - .generic_activity_with_arg("LLVM_module_codegen_asm_to_obj", &module.name[..]); - let assembly = cgcx.output_filenames.temp_path(OutputType::Assembly, module_name); - run_assembler(cgcx, diag_handler, &assembly, &obj_out); + if config_emit_normal_obj { + if !config.no_integrated_as { + let _timer = cgcx + .prof + .generic_activity_with_arg("LLVM_module_codegen_emit_obj", &module.name[..]); + with_codegen(tm, llmod, config.no_builtins, |cpm| { + write_output_file( + diag_handler, + tm, + cpm, + llmod, + &obj_out, + llvm::FileType::ObjectFile, + ) + })?; + } else { + let _timer = cgcx + .prof + .generic_activity_with_arg("LLVM_module_codegen_asm_to_obj", &module.name[..]); + let assembly = cgcx.output_filenames.temp_path(OutputType::Assembly, module_name); + run_assembler(cgcx, diag_handler, &assembly, &obj_out); - if !config.emit_asm && !cgcx.save_temps { - drop(fs::remove_file(&assembly)); + if !config.emit_asm && !cgcx.save_temps { + drop(fs::remove_file(&assembly)); + } } }