From 807d50aaa85257229812521d8d3a1edc15dbe47a Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Tue, 20 May 2025 07:12:55 -0400 Subject: [PATCH 1/3] Remove EMBED_LTO_BITCODE --- .github/workflows/release.yml | 6 +- Readme.md | 10 --- src/back/write.rs | 114 ++++++++++++---------------------- 3 files changed, 41 insertions(+), 89 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 47a40286554..9c12c063075 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -55,7 +55,7 @@ jobs: - name: Build run: | ./y.sh prepare --only-libcore - EMBED_LTO_BITCODE=1 ./y.sh build --sysroot --release --release-sysroot + ./y.sh build --sysroot --release --release-sysroot ./y.sh test --mini-tests cargo test ./y.sh clean all @@ -73,11 +73,11 @@ jobs: run: | # FIXME(antoyo): we cannot enable LTO for stdarch tests currently because of some failing LTO tests using proc-macros. echo -n 'lto = "fat"' >> build_system/build_sysroot/Cargo.toml - EMBED_LTO_BITCODE=1 ./y.sh test --release --clean --release-sysroot --build-sysroot --keep-lto-tests ${{ matrix.commands }} + ./y.sh test --release --clean --release-sysroot --build-sysroot --keep-lto-tests ${{ matrix.commands }} - name: Run y.sh cargo build run: | - EMBED_LTO_BITCODE=1 CHANNEL="release" ./y.sh cargo build --release --manifest-path tests/hello-world/Cargo.toml + CHANNEL="release" ./y.sh cargo build --release --manifest-path tests/hello-world/Cargo.toml call_found=$(objdump -dj .text tests/hello-world/target/release/hello_world | grep -c "call .*mylib.*my_func" ) ||: if [ $call_found -gt 0 ]; then echo "ERROR: call my_func found in asm" diff --git a/Readme.md b/Readme.md index 859bb1568f4..7aeb13a4da8 100644 --- a/Readme.md +++ b/Readme.md @@ -135,16 +135,6 @@ $ CHANNEL="release" $CG_GCCJIT_DIR/y.sh cargo run If you compiled cg_gccjit in debug mode (aka you didn't pass `--release` to `./y.sh test`) you should use `CHANNEL="debug"` instead or omit `CHANNEL="release"` completely. -### LTO - -To use LTO, you need to set the variable `EMBED_LTO_BITCODE=1` in addition to setting `lto = "fat"` in the `Cargo.toml`. - -Failing to set `EMBED_LTO_BITCODE` will give you the following error: - -``` -error: failed to copy bitcode to object file: No such file or directory (os error 2) -``` - ### Rustc If you want to run `rustc` directly, you can do so with: diff --git a/src/back/write.rs b/src/back/write.rs index 09e955acf39..4a767f9b077 100644 --- a/src/back/write.rs +++ b/src/back/write.rs @@ -26,11 +26,6 @@ pub(crate) fn codegen( let should_combine_object_files = module.module_llvm.should_combine_object_files; - // NOTE: Only generate object files with GIMPLE when this environment variable is set for - // now because this requires a particular setup (same gcc/lto1/lto-wrapper commit as libgccjit). - // TODO(antoyo): remove this environment variable. - let fat_lto = env::var("EMBED_LTO_BITCODE").as_deref() == Ok("1"); - let bc_out = cgcx.output_filenames.temp_path_for_cgu( OutputType::Bitcode, &module.name, @@ -43,80 +38,44 @@ pub(crate) fn codegen( ); if config.bitcode_needed() { - if fat_lto { + let _timer = cgcx + .prof + .generic_activity_with_arg("GCC_module_codegen_make_bitcode", &*module.name); + + // TODO(antoyo) + /*if let Some(bitcode_filename) = bc_out.file_name() { + cgcx.prof.artifact_size( + "llvm_bitcode", + bitcode_filename.to_string_lossy(), + data.len() as u64, + ); + }*/ + + if config.emit_bc || config.emit_obj == EmitObj::Bitcode { let _timer = cgcx .prof - .generic_activity_with_arg("GCC_module_codegen_make_bitcode", &*module.name); - - // TODO(antoyo) - /*if let Some(bitcode_filename) = bc_out.file_name() { - cgcx.prof.artifact_size( - "llvm_bitcode", - bitcode_filename.to_string_lossy(), - data.len() as u64, - ); - }*/ - - if config.emit_bc || config.emit_obj == EmitObj::Bitcode { - let _timer = cgcx.prof.generic_activity_with_arg( - "GCC_module_codegen_emit_bitcode", - &*module.name, - ); - context.add_command_line_option("-flto=auto"); - context.add_command_line_option("-flto-partition=one"); - // TODO(antoyo): remove since we don't want fat objects when it is for Bitcode only. - context.add_command_line_option("-ffat-lto-objects"); - context.compile_to_file( - OutputKind::ObjectFile, - bc_out.to_str().expect("path to str"), - ); - } - - if config.emit_obj == EmitObj::ObjectCode(BitcodeSection::Full) { - let _timer = cgcx.prof.generic_activity_with_arg( - "GCC_module_codegen_embed_bitcode", - &*module.name, - ); - // TODO(antoyo): maybe we should call embed_bitcode to have the proper iOS fixes? - //embed_bitcode(cgcx, llcx, llmod, &config.bc_cmdline, data); - - context.add_command_line_option("-flto=auto"); - context.add_command_line_option("-flto-partition=one"); - context.add_command_line_option("-ffat-lto-objects"); - // TODO(antoyo): Send -plugin/usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/liblto_plugin.so to linker (this should be done when specifying the appropriate rustc cli argument). - context.compile_to_file( - OutputKind::ObjectFile, - bc_out.to_str().expect("path to str"), - ); - } - } else { - if config.emit_bc || config.emit_obj == EmitObj::Bitcode { - let _timer = cgcx.prof.generic_activity_with_arg( - "GCC_module_codegen_emit_bitcode", - &*module.name, - ); - context.compile_to_file( - OutputKind::ObjectFile, - bc_out.to_str().expect("path to str"), - ); - } - - if config.emit_obj == EmitObj::ObjectCode(BitcodeSection::Full) { - // TODO(antoyo): we might want to emit to emit an error here, saying to set the - // environment variable EMBED_LTO_BITCODE. - let _timer = cgcx.prof.generic_activity_with_arg( - "GCC_module_codegen_embed_bitcode", - &*module.name, - ); - // TODO(antoyo): maybe we should call embed_bitcode to have the proper iOS fixes? - //embed_bitcode(cgcx, llcx, llmod, &config.bc_cmdline, data); + .generic_activity_with_arg("GCC_module_codegen_emit_bitcode", &*module.name); + context.add_command_line_option("-flto=auto"); + context.add_command_line_option("-flto-partition=one"); + // TODO(antoyo): remove since we don't want fat objects when it is for Bitcode only. + context.add_command_line_option("-ffat-lto-objects"); + context + .compile_to_file(OutputKind::ObjectFile, bc_out.to_str().expect("path to str")); + } - // TODO(antoyo): Send -plugin/usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/liblto_plugin.so to linker (this should be done when specifying the appropriate rustc cli argument). - context.compile_to_file( - OutputKind::ObjectFile, - bc_out.to_str().expect("path to str"), - ); - } + if config.emit_obj == EmitObj::ObjectCode(BitcodeSection::Full) { + let _timer = cgcx + .prof + .generic_activity_with_arg("GCC_module_codegen_embed_bitcode", &*module.name); + // TODO(antoyo): maybe we should call embed_bitcode to have the proper iOS fixes? + //embed_bitcode(cgcx, llcx, llmod, &config.bc_cmdline, data); + + context.add_command_line_option("-flto=auto"); + context.add_command_line_option("-flto-partition=one"); + context.add_command_line_option("-ffat-lto-objects"); + // TODO(antoyo): Send -plugin/usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/liblto_plugin.so to linker (this should be done when specifying the appropriate rustc cli argument). + context + .compile_to_file(OutputKind::ObjectFile, bc_out.to_str().expect("path to str")); } } @@ -166,6 +125,9 @@ pub(crate) fn codegen( context.dump_to_file(path, true); } if should_combine_object_files { + let fat_lto = config.emit_obj == EmitObj::ObjectCode(BitcodeSection::Full); + // We need to check if we're doing LTO since this code is also used for the + // dummy ThinLTO implementation to combine the object files. if fat_lto { context.add_command_line_option("-flto=auto"); context.add_command_line_option("-flto-partition=one"); From 7f02d4ec958e1c61846261f2bc103911dcde2316 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Tue, 20 May 2025 09:26:22 -0400 Subject: [PATCH 2/3] Fix to do LTO when requested --- src/back/lto.rs | 10 +++++----- src/back/write.rs | 8 ++++---- src/base.rs | 4 ++-- src/lib.rs | 11 +++++++++-- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/back/lto.rs b/src/back/lto.rs index e9c87f35779..32b18aa0530 100644 --- a/src/back/lto.rs +++ b/src/back/lto.rs @@ -40,7 +40,7 @@ use tempfile::{TempDir, tempdir}; use crate::back::write::save_temp_bitcode; use crate::errors::{DynamicLinkingWithLTO, LtoBitcodeFromRlib, LtoDisallowed, LtoDylib}; -use crate::{GccCodegenBackend, GccContext, SyncContext, to_gcc_opt_level}; +use crate::{GccCodegenBackend, GccContext, LtoMode, SyncContext, to_gcc_opt_level}; pub fn crate_type_allows_lto(crate_type: CrateType) -> bool { match crate_type { @@ -302,7 +302,7 @@ fn fat_lto( info!("linking {:?}", name); match bc_decoded { SerializedModule::Local(ref module_buffer) => { - module.module_llvm.should_combine_object_files = true; + module.module_llvm.lto_mode = LtoMode::Fat; module .module_llvm .context @@ -610,7 +610,7 @@ pub fn optimize_thin_module( // that LLVM Context and Module. //let llcx = llvm::LLVMRustContextCreate(cgcx.fewer_names); //let llmod_raw = parse_module(llcx, module_name, thin_module.data(), &dcx)? as *const _; - let mut should_combine_object_files = false; + let mut lto_mode = LtoMode::None; let context = match thin_module.shared.thin_buffers.get(thin_module.idx) { Some(thin_buffer) => Arc::clone(&thin_buffer.context), None => { @@ -621,7 +621,7 @@ pub fn optimize_thin_module( SerializedModule::Local(ref module_buffer) => { let path = module_buffer.0.to_str().expect("path"); context.add_driver_option(path); - should_combine_object_files = true; + lto_mode = LtoMode::Thin; /*module.module_llvm.should_combine_object_files = true; module .module_llvm @@ -640,7 +640,7 @@ pub fn optimize_thin_module( thin_module.name().to_string(), GccContext { context, - should_combine_object_files, + lto_mode, // TODO(antoyo): use the correct relocation model here. relocation_model: RelocModel::Pic, temp_dir: None, diff --git a/src/back/write.rs b/src/back/write.rs index 4a767f9b077..8e1aed4046b 100644 --- a/src/back/write.rs +++ b/src/back/write.rs @@ -12,7 +12,7 @@ use rustc_target::spec::SplitDebuginfo; use crate::base::add_pic_option; use crate::errors::CopyBitcode; -use crate::{GccCodegenBackend, GccContext}; +use crate::{GccCodegenBackend, GccContext, LtoMode}; pub(crate) fn codegen( cgcx: &CodegenContext, @@ -24,7 +24,7 @@ pub(crate) fn codegen( { let context = &module.module_llvm.context; - let should_combine_object_files = module.module_llvm.should_combine_object_files; + let lto_mode = module.module_llvm.lto_mode; let bc_out = cgcx.output_filenames.temp_path_for_cgu( OutputType::Bitcode, @@ -124,8 +124,8 @@ pub(crate) fn codegen( context.set_debug_info(true); context.dump_to_file(path, true); } - if should_combine_object_files { - let fat_lto = config.emit_obj == EmitObj::ObjectCode(BitcodeSection::Full); + if lto_mode != LtoMode::None { + let fat_lto = lto_mode == LtoMode::Fat; // We need to check if we're doing LTO since this code is also used for the // dummy ThinLTO implementation to combine the object files. if fat_lto { diff --git a/src/base.rs b/src/base.rs index a9d7808c833..22c221b1a03 100644 --- a/src/base.rs +++ b/src/base.rs @@ -21,7 +21,7 @@ use rustc_target::spec::{PanicStrategy, RelocModel}; use crate::builder::Builder; use crate::context::CodegenCx; -use crate::{GccContext, LockedTargetInfo, SyncContext, gcc_util, new_context}; +use crate::{GccContext, LockedTargetInfo, LtoMode, SyncContext, gcc_util, new_context}; #[cfg(feature = "master")] pub fn visibility_to_gcc(visibility: Visibility) -> gccjit::Visibility { @@ -242,7 +242,7 @@ pub fn compile_codegen_unit( GccContext { context: Arc::new(SyncContext::new(context)), relocation_model: tcx.sess.relocation_model(), - should_combine_object_files: false, + lto_mode: LtoMode::None, temp_dir: None, }, ) diff --git a/src/lib.rs b/src/lib.rs index 6e2a50d745a..65ac34e055d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -297,7 +297,7 @@ impl ExtraBackendMethods for GccCodegenBackend { let mut mods = GccContext { context: Arc::new(SyncContext::new(new_context(tcx))), relocation_model: tcx.sess.relocation_model(), - should_combine_object_files: false, + lto_mode: LtoMode::None, temp_dir: None, }; @@ -326,12 +326,19 @@ impl ExtraBackendMethods for GccCodegenBackend { } } +#[derive(Clone, Copy, PartialEq)] +pub enum LtoMode { + None, + Thin, + Fat, +} + pub struct GccContext { context: Arc, /// This field is needed in order to be able to set the flag -fPIC when necessary when doing /// LTO. relocation_model: RelocModel, - should_combine_object_files: bool, + lto_mode: LtoMode, // Temporary directory used by LTO. We keep it here so that it's not removed before linking. temp_dir: Option, } From 9c9931ae0d349c88541541157bb4f76c78767391 Mon Sep 17 00:00:00 2001 From: Antoni Boucher Date: Tue, 20 May 2025 21:15:48 -0400 Subject: [PATCH 3/3] Fix to not do LTO when LTO is not enabled in gcc --- src/back/lto.rs | 1 + src/back/write.rs | 24 +++++++++++++++--------- src/base.rs | 6 ++++-- src/lib.rs | 43 +++++++++++++++++++++++++++++++++++++------ 4 files changed, 57 insertions(+), 17 deletions(-) diff --git a/src/back/lto.rs b/src/back/lto.rs index 32b18aa0530..78e7a5beba7 100644 --- a/src/back/lto.rs +++ b/src/back/lto.rs @@ -641,6 +641,7 @@ pub fn optimize_thin_module( GccContext { context, lto_mode, + lto_supported: false, // TODO(antoyo): check if this is correct to use this value. // TODO(antoyo): use the correct relocation model here. relocation_model: RelocModel::Pic, temp_dir: None, diff --git a/src/back/write.rs b/src/back/write.rs index 8e1aed4046b..a8afe05ea8b 100644 --- a/src/back/write.rs +++ b/src/back/write.rs @@ -25,6 +25,7 @@ pub(crate) fn codegen( let context = &module.module_llvm.context; let lto_mode = module.module_llvm.lto_mode; + let lto_supported = module.module_llvm.lto_supported; let bc_out = cgcx.output_filenames.temp_path_for_cgu( OutputType::Bitcode, @@ -51,14 +52,17 @@ pub(crate) fn codegen( ); }*/ + // TODO: only emit if libgccjit is compiled with LTO enabled? if config.emit_bc || config.emit_obj == EmitObj::Bitcode { let _timer = cgcx .prof .generic_activity_with_arg("GCC_module_codegen_emit_bitcode", &*module.name); - context.add_command_line_option("-flto=auto"); - context.add_command_line_option("-flto-partition=one"); - // TODO(antoyo): remove since we don't want fat objects when it is for Bitcode only. - context.add_command_line_option("-ffat-lto-objects"); + if lto_supported { + context.add_command_line_option("-flto=auto"); + context.add_command_line_option("-flto-partition=one"); + // TODO(antoyo): remove since we don't want fat objects when it is for Bitcode only. + context.add_command_line_option("-ffat-lto-objects"); + } context .compile_to_file(OutputKind::ObjectFile, bc_out.to_str().expect("path to str")); } @@ -67,12 +71,14 @@ pub(crate) fn codegen( let _timer = cgcx .prof .generic_activity_with_arg("GCC_module_codegen_embed_bitcode", &*module.name); - // TODO(antoyo): maybe we should call embed_bitcode to have the proper iOS fixes? - //embed_bitcode(cgcx, llcx, llmod, &config.bc_cmdline, data); + if lto_supported { + // TODO(antoyo): maybe we should call embed_bitcode to have the proper iOS fixes? + //embed_bitcode(cgcx, llcx, llmod, &config.bc_cmdline, data); - context.add_command_line_option("-flto=auto"); - context.add_command_line_option("-flto-partition=one"); - context.add_command_line_option("-ffat-lto-objects"); + context.add_command_line_option("-flto=auto"); + context.add_command_line_option("-flto-partition=one"); + context.add_command_line_option("-ffat-lto-objects"); + } // TODO(antoyo): Send -plugin/usr/lib/gcc/x86_64-pc-linux-gnu/11.1.0/liblto_plugin.so to linker (this should be done when specifying the appropriate rustc cli argument). context .compile_to_file(OutputKind::ObjectFile, bc_out.to_str().expect("path to str")); diff --git a/src/base.rs b/src/base.rs index 22c221b1a03..fcaf2987e9c 100644 --- a/src/base.rs +++ b/src/base.rs @@ -74,6 +74,7 @@ pub fn compile_codegen_unit( tcx: TyCtxt<'_>, cgu_name: Symbol, target_info: LockedTargetInfo, + lto_supported: bool, ) -> (ModuleCodegen, u64) { let prof_timer = tcx.prof.generic_activity("codegen_module"); let start_time = Instant::now(); @@ -82,7 +83,7 @@ pub fn compile_codegen_unit( let (module, _) = tcx.dep_graph.with_task( dep_node, tcx, - (cgu_name, target_info), + (cgu_name, target_info, lto_supported), module_codegen, Some(dep_graph::hash_result), ); @@ -95,7 +96,7 @@ pub fn compile_codegen_unit( fn module_codegen( tcx: TyCtxt<'_>, - (cgu_name, target_info): (Symbol, LockedTargetInfo), + (cgu_name, target_info, lto_supported): (Symbol, LockedTargetInfo, bool), ) -> ModuleCodegen { let cgu = tcx.codegen_unit(cgu_name); // Instantiate monomorphizations without filling out definitions yet... @@ -242,6 +243,7 @@ pub fn compile_codegen_unit( GccContext { context: Arc::new(SyncContext::new(context)), relocation_model: tcx.sess.relocation_model(), + lto_supported, lto_mode: LtoMode::None, temp_dir: None, }, diff --git a/src/lib.rs b/src/lib.rs index 65ac34e055d..f980d33cbf2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -84,10 +84,7 @@ mod type_of; use std::any::Any; use std::fmt::Debug; use std::ops::Deref; -#[cfg(not(feature = "master"))] -use std::sync::atomic::AtomicBool; -#[cfg(not(feature = "master"))] -use std::sync::atomic::Ordering; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use back::lto::{ThinBuffer, ThinData}; @@ -181,6 +178,7 @@ impl LockedTargetInfo { #[derive(Clone)] pub struct GccCodegenBackend { target_info: LockedTargetInfo, + lto_supported: Arc, } impl CodegenBackend for GccCodegenBackend { @@ -202,6 +200,29 @@ impl CodegenBackend for GccCodegenBackend { **self.target_info.info.lock().expect("lock") = context.get_target_info(); } + // TODO: try the LTO frontend and check if it errors out. If so, do not embed the bitcode. + { + let temp_dir = TempDir::new().expect("cannot create temporary directory"); + let temp_file = temp_dir.into_path().join("result.asm"); + let context = Context::default(); + let object_file_path = temp_file.to_str().expect("path to str"); + context.compile_to_file(gccjit::OutputKind::ObjectFile, object_file_path); + + //let temp_dir = TempDir::new().expect("cannot create temporary directory"); + //let temp_file = temp_dir.into_path().join("result.asm"); + let check_context = Context::default(); + check_context.add_driver_option("-x"); + check_context.add_driver_option("lto"); + check_context.add_driver_option(object_file_path); + check_context.set_print_errors_to_stderr(false); + //context.compile_to_file(gccjit::OutputKind::ObjectFile, temp_file.to_str().expect("path to str")); + // FIXME: compile gives the error as expected, but compile_to_file doesn't. + check_context.compile(); + let error = check_context.get_last_error(); + let lto_supported = error == Ok(None); + self.lto_supported.store(lto_supported, Ordering::SeqCst); + } + #[cfg(feature = "master")] gccjit::set_global_personality_function_name(b"rust_eh_personality\0"); @@ -298,6 +319,7 @@ impl ExtraBackendMethods for GccCodegenBackend { context: Arc::new(SyncContext::new(new_context(tcx))), relocation_model: tcx.sess.relocation_model(), lto_mode: LtoMode::None, + lto_supported: false, temp_dir: None, }; @@ -312,7 +334,12 @@ impl ExtraBackendMethods for GccCodegenBackend { tcx: TyCtxt<'_>, cgu_name: Symbol, ) -> (ModuleCodegen, u64) { - base::compile_codegen_unit(tcx, cgu_name, self.target_info.clone()) + base::compile_codegen_unit( + tcx, + cgu_name, + self.target_info.clone(), + self.lto_supported.load(Ordering::SeqCst), + ) } fn target_machine_factory( @@ -339,6 +366,7 @@ pub struct GccContext { /// LTO. relocation_model: RelocModel, lto_mode: LtoMode, + lto_supported: bool, // Temporary directory used by LTO. We keep it here so that it's not removed before linking. temp_dir: Option, } @@ -475,7 +503,10 @@ pub fn __rustc_codegen_backend() -> Box { supports_128bit_integers: AtomicBool::new(false), }))); - Box::new(GccCodegenBackend { target_info: LockedTargetInfo { info } }) + Box::new(GccCodegenBackend { + lto_supported: Arc::new(AtomicBool::new(false)), + target_info: LockedTargetInfo { info }, + }) } fn to_gcc_opt_level(optlevel: Option) -> OptimizationLevel {