Skip to content

Remove -fuse-linker-plugin since we do LTO before the linker is invoked #647

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build_system/build_sysroot/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@ codegen-units = 10000

[profile.release]
debug = "limited"
#lto = "fat" # TODO(antoyo): re-enable when the failing LTO tests regarding proc-macros are fixed.
lto = "fat" # TODO(antoyo): re-enable when the failing LTO tests regarding proc-macros are fixed.
79 changes: 69 additions & 10 deletions src/back/lto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use std::fs::{self, File};
use std::path::{Path, PathBuf};
use std::sync::Arc;

use gccjit::{Context, OutputKind};
use gccjit::{Context, FnAttribute, FunctionType, GlobalKind, OutputKind};
use object::read::archive::ArchiveFile;
use rustc_codegen_ssa::back::lto::{LtoModuleCodegen, SerializedModule, ThinModule, ThinShared};
use rustc_codegen_ssa::back::symbol_export;
Expand Down Expand Up @@ -50,8 +50,7 @@ pub fn crate_type_allows_lto(crate_type: CrateType) -> bool {
}

struct LtoData {
// TODO(antoyo): use symbols_below_threshold.
//symbols_below_threshold: Vec<String>,
symbols_below_threshold: Vec<String>,
upstream_modules: Vec<(SerializedModule<ModuleBuffer>, CString)>,
tmp_path: TempDir,
}
Expand Down Expand Up @@ -156,7 +155,11 @@ fn prepare_lto(
}
}

Ok(LtoData { upstream_modules, tmp_path })
Ok(LtoData {
upstream_modules,
symbols_below_threshold,
tmp_path,
})
}

fn save_as_file(obj: &[u8], path: &Path) -> Result<(), LtoBitcodeFromRlib> {
Expand All @@ -175,16 +178,14 @@ pub(crate) fn run_fat(
let dcx = cgcx.create_dcx();
let dcx = dcx.handle();
let lto_data = prepare_lto(cgcx, dcx)?;
/*let symbols_below_threshold =
lto_data.symbols_below_threshold.iter().map(|c| c.as_ptr()).collect::<Vec<_>>();*/
fat_lto(
cgcx,
dcx,
modules,
cached_modules,
lto_data.upstream_modules,
lto_data.tmp_path,
//&lto_data.symbols_below_threshold,
&lto_data.symbols_below_threshold,
)
}

Expand All @@ -195,7 +196,7 @@ fn fat_lto(
cached_modules: Vec<(SerializedModule<ModuleBuffer>, WorkProduct)>,
mut serialized_modules: Vec<(SerializedModule<ModuleBuffer>, CString)>,
tmp_path: TempDir,
//symbols_below_threshold: &[String],
symbols_below_threshold: &[String],
) -> Result<LtoModuleCodegen<GccCodegenBackend>, FatalError> {
let _timer = cgcx.prof.generic_activity("GCC_fat_lto_build_monolithic_module");
info!("going for a fat lto");
Expand Down Expand Up @@ -271,6 +272,7 @@ fn fat_lto(
// We cannot load and merge GCC contexts in memory like cg_llvm is doing.
// Instead, we combine the object files into a single object file.
for module in in_memory {
println!("Adding -flto for {}", module.name);
let path = tmp_path.path().to_path_buf().join(&module.name);
let path = path.to_str().expect("path");
let context = &module.module_llvm.context;
Expand All @@ -279,6 +281,15 @@ fn fat_lto(
context.set_optimization_level(to_gcc_opt_level(config.opt_level));
context.add_command_line_option("-flto=auto");
context.add_command_line_option("-flto-partition=one");
context.add_driver_option("-flto=auto");
context.add_driver_option("-flto-partition=one");
// FIXME: we apparently generate fat objects since we had trouble with a sync:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix this first.

// https://blog.antoyo.xyz/rustc_codegen_gcc-progress-report-33
// TODO: Maybe fix this before removing -fuse-linker-plugin.
context.add_command_line_option("-fno-fat-lto-objects");
context.add_driver_option("-fno-fat-lto-objects");
context.add_command_line_option("-fno-use-linker-plugin");
context.add_driver_option("-fno-use-linker-plugin");
context.compile_to_file(OutputKind::ObjectFile, path);
let buffer = ModuleBuffer::new(PathBuf::from(path));
let llmod_id = CString::new(&module.name[..]).unwrap();
Expand Down Expand Up @@ -312,6 +323,45 @@ fn fat_lto(
}
save_temp_bitcode(cgcx, &module, "lto.input");

let int_type = module.module_llvm.context.new_type::<i32>();
/*
* TODO: Preserve the symbols via a linker script instead?
* TODO TODO: use the symbols in inline asm to keep them?
* ===> This seems to work and is easy.
*
* FIXME: main is not genarated in GIMPLE IR; it is generated as asm.
* ===> seems now fixed since I commented this line in libgccjit:
* ADD_ARG ("-fno-use-linker-plugin");
* FIXME: it seems "make install" doesn't copy liblto_plugin.so.
* ===> seems now fixed after a rebuild.
*
* TODO TODO TODO: add -fno-use-linker-plugin because:
* This option is enabled by default when LTO support in GCC is enabled and GCC was configured for use with a linker supporting plugins (GNU ld 2.21 or newer or gold).
*/
for symbol in symbols_below_threshold {
println!("*** Keeping symbol: {}", symbol);
module.module_llvm.context.new_global(None, GlobalKind::Imported, int_type, symbol);
// TODO: Create a function that is always_inline and that calls the symbol here (e.g.
// main)?
}
let void_type = module.module_llvm.context.new_type::<()>();
let main_func = module.module_llvm.context.new_function(None, FunctionType::Extern, void_type, &[], "main", false);
//main_func.add_attribute(FnAttribute::Used);

// NOTE: look at the code from 64b30d344ce54f8ee496cb3590b4c7cf3cb30447 to see previous
// attemps.
let func = module.module_llvm.context.new_function(None, FunctionType::Exported, void_type, &[], "__my_call_main", false);
//func.add_attribute(FnAttribute::AlwaysInline);
//func.add_attribute(FnAttribute::Inline);
func.add_attribute(FnAttribute::Used);
let block = func.new_block("start");
let main_func_address = main_func.get_address(None);
/*let asm = block.add_extended_asm(None, "");
asm.add_input_operand(None, "X", main_func_address);*/
//let call = module.module_llvm.context.new_call(None, main_func, &[]);
//block.add_eval(None, call);
block.end_with_void_return(None);

// Internalize everything below threshold to help strip out more modules and such.
/*unsafe {
let ptr = symbols_below_threshold.as_ptr();
Expand Down Expand Up @@ -476,8 +526,9 @@ fn thin_lto(
});*/

match module {
SerializedModule::Local(_) => {
//let path = module_buffer.0.to_str().expect("path");
SerializedModule::Local(ref module) => {
let path = module.0.to_str().expect("path");
println!("??? Should we add -flto for: {:?}", path);
//let my_path = PathBuf::from(path);
//let exists = my_path.exists();
/*module.module_llvm.should_combine_object_files = true;
Expand Down Expand Up @@ -611,6 +662,14 @@ pub unsafe fn optimize_thin_module(
Some(thin_buffer) => Arc::clone(&thin_buffer.context),
None => {
let context = Context::default();
context.add_command_line_option("-flto=auto");
context.add_command_line_option("-flto-partition=one");
context.add_driver_option("-flto=auto");
context.add_driver_option("-flto-partition=one");
context.add_command_line_option("-fno-fat-lto-objects");
context.add_driver_option("-fno-fat-lto-objects");
context.add_command_line_option("-fno-use-linker-plugin");
context.add_driver_option("-fno-use-linker-plugin");
let len = thin_module.shared.thin_buffers.len();
let module = &thin_module.shared.serialized_modules[thin_module.idx - len];
match *module {
Expand Down
58 changes: 53 additions & 5 deletions src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,26 @@ pub(crate) unsafe fn codegen(
);
}*/

// TODO: make this work to make it easier to debug GCC-related issue within
// rustc_codegen_gcc:
//context.add_driver_option("-wrapper gdb,--args");
if config.emit_bc || config.emit_obj == EmitObj::Bitcode {
let _timer = cgcx.prof.generic_activity_with_arg(
"GCC_module_codegen_emit_bitcode",
&*module.name,
);
println!("Adding -flto for {}", bc_out.to_str().unwrap());
context.add_command_line_option("-flto=auto");
context.add_command_line_option("-flto-partition=one");
context.add_driver_option("-flto=auto");
context.add_driver_option("-flto-partition=one");
context.add_command_line_option("-fno-fat-lto-objects");
context.add_driver_option("-fno-fat-lto-objects");
// FIXME FIXME FIXME: it seems that uncommenting "ADD_ARG ("-fno-use-linker-plugin")" in libgccjit
// make the test fail (undefined symbol main).
// TODO: Perhaps we're not sending this flag somewhere?
context.add_command_line_option("-fno-use-linker-plugin");
//context.add_driver_option("-fno-use-linker-plugin");
// 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(
Expand All @@ -80,8 +93,15 @@ pub(crate) unsafe fn codegen(
// TODO(antoyo): maybe we should call embed_bitcode to have the proper iOS fixes?
//embed_bitcode(cgcx, llcx, llmod, &config.bc_cmdline, data);

println!("Adding -flto for {}", bc_out.to_str().unwrap());
context.add_command_line_option("-flto=auto");
context.add_command_line_option("-flto-partition=one");
context.add_driver_option("-flto=auto");
context.add_driver_option("-flto-partition=one");
context.add_command_line_option("-fno-fat-lto-objects");
context.add_driver_option("-fno-fat-lto-objects");
context.add_command_line_option("-fno-use-linker-plugin");
//context.add_driver_option("-fno-use-linker-plugin");
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(
Expand Down Expand Up @@ -169,18 +189,32 @@ pub(crate) unsafe fn codegen(
if fat_lto {
context.add_command_line_option("-flto=auto");
context.add_command_line_option("-flto-partition=one");
context.add_driver_option("-flto=auto");
context.add_driver_option("-flto-partition=one");
context.add_command_line_option("-fno-fat-lto-objects");
context.add_driver_option("-fno-fat-lto-objects");
//context.add_command_line_option("-ffat-lto-objects");
context.add_command_line_option("-fno-use-linker-plugin");
//context.add_driver_option("-fno-use-linker-plugin");

// NOTE: without -fuse-linker-plugin, we get the following error:
// lto1: internal compiler error: decompressed stream: Destination buffer is too small
// TODO(antoyo): since we do not do LTO when the linker is invoked anymore, perhaps
// the following flag is not necessary anymore.
context.add_driver_option("-fuse-linker-plugin");
// FIXME:
// /usr/bin/ld: warning: incremental linking of LTO and non-LTO objects; using -flinker-output=nolto-rel which will bypass whole program optimization
// => So I'm probably missing -flto somewhere.
// ====> That was caused because I didn't build the sysroot with LTO.

println!("Adding -flto to {:?}", obj_out.to_str().expect("path to str"));

// FIXME: the problem is probably that the code is only in GIMPLE IR while
// we would want to get the optimized asm done from LTO.
// ===> But we call "gcc -x lto" later, so that should be asm and not
// GIMPLE IR.
}

context.add_driver_option("-Wl,-r");
// NOTE: we need -nostdlib, otherwise, we get the following error:
// /usr/bin/ld: cannot find -lgcc_s: No such file or directory
context.add_driver_option("-nostdlib");
//context.add_driver_option("-v");

let path = obj_out.to_str().expect("path to str");

Expand All @@ -194,11 +228,22 @@ pub(crate) unsafe fn codegen(
//
// This option is to mute it to make the UI tests pass with LTO enabled.
context.add_driver_option("-Wno-lto-type-mismatch");
//context.add_driver_option("-v");
// NOTE: this doesn't actually generate an executable. With the above
// flags, it combines the .o files together in another .o.
println!("Lto path: {:?}", lto_path);
context.compile_to_file(OutputKind::Executable, &lto_path);
println!("****************************************************************************************************");

let context = Context::default();
context.add_command_line_option("-flto=auto");
context.add_command_line_option("-flto-partition=one");
context.add_driver_option("-flto=auto");
context.add_driver_option("-flto-partition=one");
context.add_command_line_option("-fno-fat-lto-objects");
context.add_driver_option("-fno-fat-lto-objects");
context.add_command_line_option("-fno-use-linker-plugin");
//context.add_driver_option("-fno-use-linker-plugin");
if cgcx.target_arch == "x86" || cgcx.target_arch == "x86_64" {
// NOTE: it seems we need to use add_driver_option instead of
// add_command_line_option here because we use the LTO frontend via gcc.
Expand All @@ -210,8 +255,11 @@ pub(crate) unsafe fn codegen(
// needs to be at the very beginning.
context.add_driver_option("-x");
context.add_driver_option("lto");
//context.add_driver_option("-v");
add_pic_option(&context, module.module_llvm.relocation_model);
context.add_driver_option(lto_path);
// TODO TODO: inspect the resulting file to see if it contains the GIMPLE IR or
// the asm.

context.compile_to_file(OutputKind::ObjectFile, path);
} else {
Expand Down
2 changes: 2 additions & 0 deletions src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ pub fn compile_codegen_unit(
let cgu = tcx.codegen_unit(cgu_name);
// Instantiate monomorphizations without filling out definitions yet...
let context = new_context(tcx);
println!("==== Module codegen: {}", cgu_name);
context.add_driver_option("-fno-use-linker-plugin");

if tcx.sess.panic_strategy() == PanicStrategy::Unwind {
context.add_command_line_option("-fexceptions");
Expand Down
4 changes: 4 additions & 0 deletions src/declare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,10 @@ fn declare_raw_fn<'gcc>(
.collect();
#[cfg(not(feature = "master"))]
let name = &mangle_name(name);
if name == "main" {
//cx.context.add_driver_option("-v");
println!("**** Main function in {}", cx.codegen_unit.name());
}
let func =
cx.context.new_function(None, cx.linkage.get(), return_type, &params, name, variadic);
#[cfg(feature = "master")]
Expand Down
2 changes: 2 additions & 0 deletions src/gcc_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,8 @@ fn handle_native(name: &str) -> &str {
{
// Get the native arch.
let context = Context::default();
context.add_command_line_option("-fno-use-linker-plugin");
context.add_driver_option("-fno-use-linker-plugin");
context.get_target_info().arch().unwrap().to_str().unwrap()
}
#[cfg(not(feature = "master"))]
Expand Down
13 changes: 13 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@ impl CodegenBackend for GccCodegenBackend {

// Get the second TargetInfo with the correct CPU features by setting the arch.
let context = Context::default();
context.add_command_line_option("-fno-use-linker-plugin");
//context.add_driver_option("-fno-use-linker-plugin");
if target_cpu != "generic" {
context.add_command_line_option(format!("-march={}", target_cpu));
}
Expand All @@ -210,6 +212,8 @@ impl CodegenBackend for GccCodegenBackend {
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();
context.add_command_line_option("-fno-use-linker-plugin");
//context.add_driver_option("-fno-use-linker-plugin");
check_context.set_print_errors_to_stderr(false);
let _int128_ty = check_context.new_c_type(CType::UInt128t);
// NOTE: we cannot just call compile() as this would require other files than libgccjit.so.
Expand Down Expand Up @@ -267,6 +271,12 @@ impl CodegenBackend for GccCodegenBackend {

fn new_context<'gcc, 'tcx>(tcx: TyCtxt<'tcx>) -> Context<'gcc> {
let context = Context::default();
context.add_command_line_option("-flto=auto");
context.add_command_line_option("-flto-partition=one");
context.add_driver_option("-flto=auto");
context.add_driver_option("-flto-partition=one");
context.add_command_line_option("-fno-use-linker-plugin");
//context.add_driver_option("-fno-use-linker-plugin");
if tcx.sess.target.arch == "x86" || tcx.sess.target.arch == "x86_64" {
context.add_command_line_option("-masm=intel");
}
Expand Down Expand Up @@ -300,6 +310,7 @@ impl ExtraBackendMethods for GccCodegenBackend {
should_combine_object_files: false,
temp_dir: None,
};
mods.context.add_driver_option("-fno-use-linker-plugin");

unsafe {
allocator::codegen(tcx, &mut mods, module_name, kind, alloc_error_handler_kind);
Expand Down Expand Up @@ -461,6 +472,8 @@ pub fn __rustc_codegen_backend() -> Box<dyn CodegenBackend> {
// Check whether the target supports 128-bit integers, and sized floating point types (like
// Float16).
let context = Context::default();
context.add_command_line_option("-fno-use-linker-plugin");
//context.add_driver_option("-fno-use-linker-plugin");
Arc::new(Mutex::new(IntoDynSyncSend(context.get_target_info())))
};
#[cfg(not(feature = "master"))]
Expand Down
Loading