From 701b2747c331fb311ab67e50519bb226d86b76dc Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Wed, 17 Aug 2022 13:03:32 +0000 Subject: [PATCH 01/15] Remove optimize_function It currently doesn't have any optimizations at all. --- src/base.rs | 13 +------------ src/optimize/mod.rs | 17 ----------------- 2 files changed, 1 insertion(+), 29 deletions(-) diff --git a/src/base.rs b/src/base.rs index c68d33465..2603da191 100644 --- a/src/base.rs +++ b/src/base.rs @@ -148,7 +148,7 @@ fn compile_fn<'tcx>( ) { let tcx = cx.tcx; - let mut clif_comments = codegened_func.clif_comments; + let clif_comments = codegened_func.clif_comments; // Store function in context let context = cached_context; @@ -165,17 +165,6 @@ fn compile_fn<'tcx>( // invalidate it when it would change. context.domtree.clear(); - // Perform rust specific optimizations - tcx.sess.time("optimize clif ir", || { - crate::optimize::optimize_function( - tcx, - module.isa(), - codegened_func.instance, - context, - &mut clif_comments, - ); - }); - #[cfg(any())] // This is never true let _clif_guard = { use std::fmt::Write; diff --git a/src/optimize/mod.rs b/src/optimize/mod.rs index d1f89adb3..0df7e8229 100644 --- a/src/optimize/mod.rs +++ b/src/optimize/mod.rs @@ -1,20 +1,3 @@ //! Various optimizations specific to cg_clif -use cranelift_codegen::isa::TargetIsa; - -use crate::prelude::*; - pub(crate) mod peephole; - -pub(crate) fn optimize_function<'tcx>( - tcx: TyCtxt<'tcx>, - isa: &dyn TargetIsa, - instance: Instance<'tcx>, - ctx: &mut Context, - clif_comments: &mut crate::pretty_clif::CommentWriter, -) { - // FIXME classify optimizations over opt levels once we have more - - crate::pretty_clif::write_clif_file(tcx, "preopt", isa, instance, &ctx.func, &*clif_comments); - crate::base::verify_func(tcx, &*clif_comments, &ctx.func); -} From b181f2b3769f1185d328efb1569c3da72e5edaa0 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Wed, 17 Aug 2022 13:07:18 +0000 Subject: [PATCH 02/15] Replace instance param of write_clif_file with symbol_name --- src/base.rs | 6 +++--- src/pretty_clif.rs | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/base.rs b/src/base.rs index 2603da191..85cdc6824 100644 --- a/src/base.rs +++ b/src/base.rs @@ -119,9 +119,9 @@ fn codegen_fn<'tcx>( crate::pretty_clif::write_clif_file( tcx, + symbol_name.name, "unopt", module.isa(), - instance, &func, &clif_comments, ); @@ -201,9 +201,9 @@ fn compile_fn<'tcx>( // Write optimized function to file for debugging crate::pretty_clif::write_clif_file( tcx, + codegened_func.symbol_name.name, "opt", module.isa(), - codegened_func.instance, &context.func, &clif_comments, ); @@ -211,7 +211,7 @@ fn compile_fn<'tcx>( if let Some(disasm) = &context.mach_compile_result.as_ref().unwrap().disasm { crate::pretty_clif::write_ir_file( tcx, - || format!("{}.vcode", tcx.symbol_name(codegened_func.instance).name), + || format!("{}.vcode", codegened_func.symbol_name.name), |file| file.write_all(disasm.as_bytes()), ) } diff --git a/src/pretty_clif.rs b/src/pretty_clif.rs index 1d1ec2168..0081ec842 100644 --- a/src/pretty_clif.rs +++ b/src/pretty_clif.rs @@ -231,16 +231,16 @@ pub(crate) fn write_ir_file( pub(crate) fn write_clif_file<'tcx>( tcx: TyCtxt<'tcx>, + symbol_name: &str, postfix: &str, isa: &dyn cranelift_codegen::isa::TargetIsa, - instance: Instance<'tcx>, func: &cranelift_codegen::ir::Function, mut clif_comments: &CommentWriter, ) { // FIXME work around filename too long errors write_ir_file( tcx, - || format!("{}.{}.clif", tcx.symbol_name(instance).name, postfix), + || format!("{}.{}.clif", symbol_name, postfix), |file| { let mut clif = String::new(); cranelift_codegen::write::decorate_function(&mut clif_comments, &mut clif, func) From c820b7cd607cd2fa7eec88785d92d10461b39053 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Wed, 17 Aug 2022 13:43:32 +0000 Subject: [PATCH 03/15] Remove TyCtxt field from CodegenCx --- src/base.rs | 66 ++++++++++++++++++++++++---------------------- src/driver/aot.rs | 3 ++- src/driver/jit.rs | 10 +++---- src/lib.rs | 7 +++-- src/pretty_clif.rs | 59 +++++++++++++++++++---------------------- 5 files changed, 73 insertions(+), 72 deletions(-) diff --git a/src/base.rs b/src/base.rs index 85cdc6824..6a276df40 100644 --- a/src/base.rs +++ b/src/base.rs @@ -24,22 +24,23 @@ struct CodegenedFunction<'tcx> { } pub(crate) fn codegen_and_compile_fn<'tcx>( + tcx: TyCtxt<'tcx>, cx: &mut crate::CodegenCx<'tcx>, cached_context: &mut Context, module: &mut dyn Module, instance: Instance<'tcx>, ) { - let tcx = cx.tcx; let _inst_guard = crate::PrintOnPanic(|| format!("{:?} {}", instance, tcx.symbol_name(instance).name)); let cached_func = std::mem::replace(&mut cached_context.func, Function::new()); - let codegened_func = codegen_fn(cx, cached_func, module, instance); + let codegened_func = codegen_fn(tcx, cx, cached_func, module, instance); - compile_fn(cx, cached_context, module, codegened_func); + compile_fn(tcx, cx, cached_context, module, codegened_func); } fn codegen_fn<'tcx>( + tcx: TyCtxt<'tcx>, cx: &mut crate::CodegenCx<'tcx>, cached_func: Function, module: &mut dyn Module, @@ -47,8 +48,6 @@ fn codegen_fn<'tcx>( ) -> CodegenedFunction<'tcx> { debug_assert!(!instance.substs.needs_infer()); - let tcx = cx.tcx; - let mir = tcx.instance_mir(instance.def); let _mir_guard = crate::PrintOnPanic(|| { let mut buf = Vec::new(); @@ -117,14 +116,16 @@ fn codegen_fn<'tcx>( fx.constants_cx.finalize(fx.tcx, &mut *fx.module); - crate::pretty_clif::write_clif_file( - tcx, - symbol_name.name, - "unopt", - module.isa(), - &func, - &clif_comments, - ); + if cx.should_write_ir { + crate::pretty_clif::write_clif_file( + tcx.output_filenames(()), + symbol_name.name, + "unopt", + module.isa(), + &func, + &clif_comments, + ); + } // Verify function verify_func(tcx, &clif_comments, &func); @@ -141,13 +142,12 @@ fn codegen_fn<'tcx>( } fn compile_fn<'tcx>( + tcx: TyCtxt<'tcx>, cx: &mut crate::CodegenCx<'tcx>, cached_context: &mut Context, module: &mut dyn Module, codegened_func: CodegenedFunction<'tcx>, ) { - let tcx = cx.tcx; - let clif_comments = codegened_func.clif_comments; // Store function in context @@ -194,26 +194,28 @@ fn compile_fn<'tcx>( // Define function tcx.sess.time("define function", || { - context.want_disasm = crate::pretty_clif::should_write_ir(tcx); + context.want_disasm = cx.should_write_ir; module.define_function(codegened_func.func_id, context).unwrap(); }); - // Write optimized function to file for debugging - crate::pretty_clif::write_clif_file( - tcx, - codegened_func.symbol_name.name, - "opt", - module.isa(), - &context.func, - &clif_comments, - ); - - if let Some(disasm) = &context.mach_compile_result.as_ref().unwrap().disasm { - crate::pretty_clif::write_ir_file( - tcx, - || format!("{}.vcode", codegened_func.symbol_name.name), - |file| file.write_all(disasm.as_bytes()), - ) + if cx.should_write_ir { + // Write optimized function to file for debugging + crate::pretty_clif::write_clif_file( + &cx.output_filenames, + codegened_func.symbol_name.name, + "opt", + module.isa(), + &context.func, + &clif_comments, + ); + + if let Some(disasm) = &context.mach_compile_result.as_ref().unwrap().disasm { + crate::pretty_clif::write_ir_file( + &cx.output_filenames, + &format!("{}.vcode", codegened_func.symbol_name.name), + |file| file.write_all(disasm.as_bytes()), + ) + } } // Define debuginfo for function diff --git a/src/driver/aot.rs b/src/driver/aot.rs index 9d819e399..b2ad3cc4c 100644 --- a/src/driver/aot.rs +++ b/src/driver/aot.rs @@ -256,8 +256,9 @@ fn module_codegen( for (mono_item, _) in mono_items { match mono_item { MonoItem::Fn(inst) => { - cx.tcx.sess.time("codegen fn", || { + tcx.sess.time("codegen fn", || { crate::base::codegen_and_compile_fn( + tcx, &mut cx, &mut cached_context, &mut module, diff --git a/src/driver/jit.rs b/src/driver/jit.rs index 1b046d7ec..de71b76da 100644 --- a/src/driver/jit.rs +++ b/src/driver/jit.rs @@ -129,8 +129,9 @@ pub(crate) fn run_jit(tcx: TyCtxt<'_>, backend_config: BackendConfig) -> ! { MonoItem::Fn(inst) => match backend_config.codegen_mode { CodegenMode::Aot => unreachable!(), CodegenMode::Jit => { - cx.tcx.sess.time("codegen fn", || { + tcx.sess.time("codegen fn", || { crate::base::codegen_and_compile_fn( + tcx, &mut cx, &mut cached_context, &mut jit_module, @@ -139,7 +140,7 @@ pub(crate) fn run_jit(tcx: TyCtxt<'_>, backend_config: BackendConfig) -> ! { }); } CodegenMode::JitLazy => { - codegen_shim(&mut cx, &mut cached_context, &mut jit_module, inst) + codegen_shim(tcx, &mut cached_context, &mut jit_module, inst) } }, MonoItem::Static(def_id) => { @@ -269,6 +270,7 @@ fn jit_fn(instance_ptr: *const Instance<'static>, trampoline_ptr: *const u8) -> ); tcx.sess.time("codegen fn", || { crate::base::codegen_and_compile_fn( + tcx, &mut cx, &mut Context::new(), jit_module, @@ -350,13 +352,11 @@ fn load_imported_symbols_for_jit( } fn codegen_shim<'tcx>( - cx: &mut CodegenCx<'tcx>, + tcx: TyCtxt<'tcx>, cached_context: &mut Context, module: &mut JITModule, inst: Instance<'tcx>, ) { - let tcx = cx.tcx; - let pointer_type = module.target_config().pointer_type(); let name = tcx.symbol_name(inst).name; diff --git a/src/lib.rs b/src/lib.rs index 909f4f00f..1e851e31a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,6 +26,7 @@ extern crate rustc_driver; use std::any::Any; use std::cell::{Cell, RefCell}; +use std::sync::Arc; use rustc_codegen_ssa::traits::CodegenBackend; use rustc_codegen_ssa::CodegenResults; @@ -121,7 +122,8 @@ impl String> Drop for PrintOnPanic { /// The codegen context holds any information shared between the codegen of individual functions /// inside a single codegen unit with the exception of the Cranelift [`Module`](cranelift_module::Module). struct CodegenCx<'tcx> { - tcx: TyCtxt<'tcx>, + output_filenames: Arc, + should_write_ir: bool, global_asm: String, inline_asm_index: Cell, debug_context: Option>, @@ -147,7 +149,8 @@ impl<'tcx> CodegenCx<'tcx> { None }; CodegenCx { - tcx, + output_filenames: tcx.output_filenames(()).clone(), + should_write_ir: crate::pretty_clif::should_write_ir(tcx), global_asm: String::new(), inline_asm_index: Cell::new(0), debug_context, diff --git a/src/pretty_clif.rs b/src/pretty_clif.rs index 0081ec842..a7af16268 100644 --- a/src/pretty_clif.rs +++ b/src/pretty_clif.rs @@ -62,7 +62,7 @@ use cranelift_codegen::{ }; use rustc_middle::ty::layout::FnAbiOf; -use rustc_session::config::OutputType; +use rustc_session::config::{OutputFilenames, OutputType}; use crate::prelude::*; @@ -205,15 +205,11 @@ pub(crate) fn should_write_ir(tcx: TyCtxt<'_>) -> bool { } pub(crate) fn write_ir_file( - tcx: TyCtxt<'_>, - name: impl FnOnce() -> String, + output_filenames: &OutputFilenames, + name: &str, write: impl FnOnce(&mut dyn Write) -> std::io::Result<()>, ) { - if !should_write_ir(tcx) { - return; - } - - let clif_output_dir = tcx.output_filenames(()).with_extension("clif"); + let clif_output_dir = output_filenames.with_extension("clif"); match std::fs::create_dir(&clif_output_dir) { Ok(()) => {} @@ -221,16 +217,20 @@ pub(crate) fn write_ir_file( res @ Err(_) => res.unwrap(), } - let clif_file_name = clif_output_dir.join(name()); + let clif_file_name = clif_output_dir.join(name); let res = std::fs::File::create(clif_file_name).and_then(|mut file| write(&mut file)); if let Err(err) = res { - tcx.sess.warn(&format!("error writing ir file: {}", err)); + // Using early_warn as no Session is available here + rustc_session::early_warn( + rustc_session::config::ErrorOutputType::default(), + &format!("error writing ir file: {}", err), + ); } } -pub(crate) fn write_clif_file<'tcx>( - tcx: TyCtxt<'tcx>, +pub(crate) fn write_clif_file( + output_filenames: &OutputFilenames, symbol_name: &str, postfix: &str, isa: &dyn cranelift_codegen::isa::TargetIsa, @@ -238,27 +238,22 @@ pub(crate) fn write_clif_file<'tcx>( mut clif_comments: &CommentWriter, ) { // FIXME work around filename too long errors - write_ir_file( - tcx, - || format!("{}.{}.clif", symbol_name, postfix), - |file| { - let mut clif = String::new(); - cranelift_codegen::write::decorate_function(&mut clif_comments, &mut clif, func) - .unwrap(); + write_ir_file(output_filenames, &format!("{}.{}.clif", symbol_name, postfix), |file| { + let mut clif = String::new(); + cranelift_codegen::write::decorate_function(&mut clif_comments, &mut clif, func).unwrap(); - for flag in isa.flags().iter() { - writeln!(file, "set {}", flag)?; - } - write!(file, "target {}", isa.triple().architecture.to_string())?; - for isa_flag in isa.isa_flags().iter() { - write!(file, " {}", isa_flag)?; - } - writeln!(file, "\n")?; - writeln!(file)?; - file.write_all(clif.as_bytes())?; - Ok(()) - }, - ); + for flag in isa.flags().iter() { + writeln!(file, "set {}", flag)?; + } + write!(file, "target {}", isa.triple().architecture.to_string())?; + for isa_flag in isa.isa_flags().iter() { + write!(file, " {}", isa_flag)?; + } + writeln!(file, "\n")?; + writeln!(file)?; + file.write_all(clif.as_bytes())?; + Ok(()) + }); } impl fmt::Debug for FunctionCx<'_, '_, '_> { From df1b25171c8b2b209d1c667ea8a2e41a81c108f2 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Wed, 17 Aug 2022 13:47:52 +0000 Subject: [PATCH 04/15] Remove TyCtxt parameter from compile_fn --- src/base.rs | 7 +++---- src/lib.rs | 3 +++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/base.rs b/src/base.rs index 6a276df40..a7c7f66e1 100644 --- a/src/base.rs +++ b/src/base.rs @@ -36,7 +36,7 @@ pub(crate) fn codegen_and_compile_fn<'tcx>( let cached_func = std::mem::replace(&mut cached_context.func, Function::new()); let codegened_func = codegen_fn(tcx, cx, cached_func, module, instance); - compile_fn(tcx, cx, cached_context, module, codegened_func); + compile_fn(cx, cached_context, module, codegened_func); } fn codegen_fn<'tcx>( @@ -142,7 +142,6 @@ fn codegen_fn<'tcx>( } fn compile_fn<'tcx>( - tcx: TyCtxt<'tcx>, cx: &mut crate::CodegenCx<'tcx>, cached_context: &mut Context, module: &mut dyn Module, @@ -193,7 +192,7 @@ fn compile_fn<'tcx>( }; // Define function - tcx.sess.time("define function", || { + cx.profiler.verbose_generic_activity("define function").run(|| { context.want_disasm = cx.should_write_ir; module.define_function(codegened_func.func_id, context).unwrap(); }); @@ -222,7 +221,7 @@ fn compile_fn<'tcx>( let isa = module.isa(); let debug_context = &mut cx.debug_context; let unwind_context = &mut cx.unwind_context; - tcx.sess.time("generate debug info", || { + cx.profiler.verbose_generic_activity("generate debug info").run(|| { if let Some(debug_context) = debug_context { debug_context.define_function( codegened_func.instance, diff --git a/src/lib.rs b/src/lib.rs index 1e851e31a..f4ffbbcf8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -30,6 +30,7 @@ use std::sync::Arc; use rustc_codegen_ssa::traits::CodegenBackend; use rustc_codegen_ssa::CodegenResults; +use rustc_data_structures::profiling::SelfProfilerRef; use rustc_errors::ErrorGuaranteed; use rustc_metadata::EncodedMetadata; use rustc_middle::dep_graph::{WorkProduct, WorkProductId}; @@ -122,6 +123,7 @@ impl String> Drop for PrintOnPanic { /// The codegen context holds any information shared between the codegen of individual functions /// inside a single codegen unit with the exception of the Cranelift [`Module`](cranelift_module::Module). struct CodegenCx<'tcx> { + profiler: SelfProfilerRef, output_filenames: Arc, should_write_ir: bool, global_asm: String, @@ -149,6 +151,7 @@ impl<'tcx> CodegenCx<'tcx> { None }; CodegenCx { + profiler: tcx.prof.clone(), output_filenames: tcx.output_filenames(()).clone(), should_write_ir: crate::pretty_clif::should_write_ir(tcx), global_asm: String::new(), From 5fc1366dfa0b5c9615be028e75f4f237f165831b Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 18 Aug 2022 12:55:44 +0000 Subject: [PATCH 05/15] Register debuginfo for lazy jit shim --- src/driver/jit.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/driver/jit.rs b/src/driver/jit.rs index de71b76da..00e726344 100644 --- a/src/driver/jit.rs +++ b/src/driver/jit.rs @@ -140,7 +140,7 @@ pub(crate) fn run_jit(tcx: TyCtxt<'_>, backend_config: BackendConfig) -> ! { }); } CodegenMode::JitLazy => { - codegen_shim(tcx, &mut cached_context, &mut jit_module, inst) + codegen_shim(tcx, &mut cx, &mut cached_context, &mut jit_module, inst) } }, MonoItem::Static(def_id) => { @@ -353,6 +353,7 @@ fn load_imported_symbols_for_jit( fn codegen_shim<'tcx>( tcx: TyCtxt<'tcx>, + cx: &mut CodegenCx<'tcx>, cached_context: &mut Context, module: &mut JITModule, inst: Instance<'tcx>, @@ -403,4 +404,5 @@ fn codegen_shim<'tcx>( trampoline_builder.ins().return_(&ret_vals); module.define_function(func_id, context).unwrap(); + cx.unwind_context.add_function(func_id, context, module.isa()); } From a49416da6ddcbcbd40f7aadb06b71d77c23e8220 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 18 Aug 2022 15:14:04 +0000 Subject: [PATCH 06/15] Remove stub local debuginfo implementation It isn't actually wired up and temporarily removing it will make changes to the debuginfo generation code much simpler. --- src/base.rs | 14 +-- src/debuginfo/mod.rs | 230 +------------------------------------------ 2 files changed, 5 insertions(+), 239 deletions(-) diff --git a/src/base.rs b/src/base.rs index a7c7f66e1..fe04567af 100644 --- a/src/base.rs +++ b/src/base.rs @@ -20,7 +20,6 @@ struct CodegenedFunction<'tcx> { func: Function, clif_comments: CommentWriter, source_info_set: IndexSet, - local_map: IndexVec>, } pub(crate) fn codegen_and_compile_fn<'tcx>( @@ -112,7 +111,6 @@ fn codegen_fn<'tcx>( let instance = fx.instance; let clif_comments = fx.clif_comments; let source_info_set = fx.source_info_set; - let local_map = fx.local_map; fx.constants_cx.finalize(fx.tcx, &mut *fx.module); @@ -130,15 +128,7 @@ fn codegen_fn<'tcx>( // Verify function verify_func(tcx, &clif_comments, &func); - CodegenedFunction { - instance, - symbol_name, - func_id, - func, - clif_comments, - source_info_set, - local_map, - } + CodegenedFunction { instance, symbol_name, func_id, func, clif_comments, source_info_set } } fn compile_fn<'tcx>( @@ -227,10 +217,8 @@ fn compile_fn<'tcx>( codegened_func.instance, codegened_func.func_id, codegened_func.symbol_name.name, - isa, context, &codegened_func.source_info_set, - codegened_func.local_map, ); } unwind_context.add_function(codegened_func.func_id, &context, isa); diff --git a/src/debuginfo/mod.rs b/src/debuginfo/mod.rs index 693092ba5..7a330ff7d 100644 --- a/src/debuginfo/mod.rs +++ b/src/debuginfo/mod.rs @@ -7,18 +7,11 @@ mod unwind; use crate::prelude::*; -use rustc_index::vec::IndexVec; - -use cranelift_codegen::entity::EntityRef; -use cranelift_codegen::ir::{Endianness, LabelValueLoc, ValueLabel}; +use cranelift_codegen::ir::Endianness; use cranelift_codegen::isa::TargetIsa; -use cranelift_codegen::ValueLocRange; -use gimli::write::{ - Address, AttributeValue, DwarfUnit, Expression, LineProgram, LineString, Location, - LocationList, Range, RangeList, UnitEntryId, -}; -use gimli::{Encoding, Format, LineEncoding, RunTimeEndian, X86_64}; +use gimli::write::{Address, AttributeValue, DwarfUnit, LineProgram, LineString, Range, RangeList}; +use gimli::{Encoding, Format, LineEncoding, RunTimeEndian}; pub(crate) use emit::{DebugReloc, DebugRelocName}; pub(crate) use unwind::UnwindContext; @@ -30,8 +23,6 @@ pub(crate) struct DebugContext<'tcx> { dwarf: DwarfUnit, unit_range_list: RangeList, - - types: FxHashMap, UnitEntryId>, } impl<'tcx> DebugContext<'tcx> { @@ -101,113 +92,7 @@ impl<'tcx> DebugContext<'tcx> { root.set(gimli::DW_AT_low_pc, AttributeValue::Address(Address::Constant(0))); } - DebugContext { - tcx, - - endian, - - dwarf, - unit_range_list: RangeList(Vec::new()), - - types: FxHashMap::default(), - } - } - - fn dwarf_ty(&mut self, ty: Ty<'tcx>) -> UnitEntryId { - if let Some(type_id) = self.types.get(&ty) { - return *type_id; - } - - let new_entry = |dwarf: &mut DwarfUnit, tag| dwarf.unit.add(dwarf.unit.root(), tag); - - let primitive = |dwarf: &mut DwarfUnit, ate| { - let type_id = new_entry(dwarf, gimli::DW_TAG_base_type); - let type_entry = dwarf.unit.get_mut(type_id); - type_entry.set(gimli::DW_AT_encoding, AttributeValue::Encoding(ate)); - type_id - }; - - let name = format!("{}", ty); - let layout = self.tcx.layout_of(ParamEnv::reveal_all().and(ty)).unwrap(); - - let type_id = match ty.kind() { - ty::Bool => primitive(&mut self.dwarf, gimli::DW_ATE_boolean), - ty::Char => primitive(&mut self.dwarf, gimli::DW_ATE_UTF), - ty::Uint(_) => primitive(&mut self.dwarf, gimli::DW_ATE_unsigned), - ty::Int(_) => primitive(&mut self.dwarf, gimli::DW_ATE_signed), - ty::Float(_) => primitive(&mut self.dwarf, gimli::DW_ATE_float), - ty::Ref(_, pointee_ty, _mutbl) - | ty::RawPtr(ty::TypeAndMut { ty: pointee_ty, mutbl: _mutbl }) => { - let type_id = new_entry(&mut self.dwarf, gimli::DW_TAG_pointer_type); - - // Ensure that type is inserted before recursing to avoid duplicates - self.types.insert(ty, type_id); - - let pointee = self.dwarf_ty(*pointee_ty); - - let type_entry = self.dwarf.unit.get_mut(type_id); - - //type_entry.set(gimli::DW_AT_mutable, AttributeValue::Flag(mutbl == rustc_hir::Mutability::Mut)); - type_entry.set(gimli::DW_AT_type, AttributeValue::UnitRef(pointee)); - - type_id - } - ty::Adt(adt_def, _substs) if adt_def.is_struct() && !layout.is_unsized() => { - let type_id = new_entry(&mut self.dwarf, gimli::DW_TAG_structure_type); - - // Ensure that type is inserted before recursing to avoid duplicates - self.types.insert(ty, type_id); - - let variant = adt_def.non_enum_variant(); - - for (field_idx, field_def) in variant.fields.iter().enumerate() { - let field_offset = layout.fields.offset(field_idx); - let field_layout = layout.field( - &layout::LayoutCx { tcx: self.tcx, param_env: ParamEnv::reveal_all() }, - field_idx, - ); - - let field_type = self.dwarf_ty(field_layout.ty); - - let field_id = self.dwarf.unit.add(type_id, gimli::DW_TAG_member); - let field_entry = self.dwarf.unit.get_mut(field_id); - - field_entry.set( - gimli::DW_AT_name, - AttributeValue::String(field_def.name.as_str().to_string().into_bytes()), - ); - field_entry.set( - gimli::DW_AT_data_member_location, - AttributeValue::Udata(field_offset.bytes()), - ); - field_entry.set(gimli::DW_AT_type, AttributeValue::UnitRef(field_type)); - } - - type_id - } - _ => new_entry(&mut self.dwarf, gimli::DW_TAG_structure_type), - }; - - let type_entry = self.dwarf.unit.get_mut(type_id); - - type_entry.set(gimli::DW_AT_name, AttributeValue::String(name.into_bytes())); - type_entry.set(gimli::DW_AT_byte_size, AttributeValue::Udata(layout.size.bytes())); - - self.types.insert(ty, type_id); - - type_id - } - - fn define_local(&mut self, scope: UnitEntryId, name: String, ty: Ty<'tcx>) -> UnitEntryId { - let dw_ty = self.dwarf_ty(ty); - - let var_id = self.dwarf.unit.add(scope, gimli::DW_TAG_variable); - let var_entry = self.dwarf.unit.get_mut(var_id); - - var_entry.set(gimli::DW_AT_name, AttributeValue::String(name.into_bytes())); - var_entry.set(gimli::DW_AT_type, AttributeValue::UnitRef(dw_ty)); - - var_id + DebugContext { tcx, endian, dwarf, unit_range_list: RangeList(Vec::new()) } } pub(crate) fn define_function( @@ -215,10 +100,8 @@ impl<'tcx> DebugContext<'tcx> { instance: Instance<'tcx>, func_id: FuncId, name: &str, - isa: &dyn TargetIsa, context: &Context, source_info_set: &indexmap::IndexSet, - local_map: IndexVec>, ) { let symbol = func_id.as_u32() as usize; let mir = self.tcx.instance_mir(instance.def); @@ -248,110 +131,5 @@ impl<'tcx> DebugContext<'tcx> { ); // Using Udata for DW_AT_high_pc requires at least DWARF4 func_entry.set(gimli::DW_AT_high_pc, AttributeValue::Udata(u64::from(end))); - - // FIXME make it more reliable and implement scopes before re-enabling this. - if false { - let value_labels_ranges = std::collections::HashMap::new(); // FIXME - - for (local, _local_decl) in mir.local_decls.iter_enumerated() { - let ty = self.tcx.subst_and_normalize_erasing_regions( - instance.substs, - ty::ParamEnv::reveal_all(), - mir.local_decls[local].ty, - ); - let var_id = self.define_local(entry_id, format!("{:?}", local), ty); - - let location = place_location( - self, - isa, - symbol, - &local_map, - &value_labels_ranges, - Place { local, projection: ty::List::empty() }, - ); - - let var_entry = self.dwarf.unit.get_mut(var_id); - var_entry.set(gimli::DW_AT_location, location); - } - } - - // FIXME create locals for all entries in mir.var_debug_info - } -} - -fn place_location<'tcx>( - debug_context: &mut DebugContext<'tcx>, - isa: &dyn TargetIsa, - symbol: usize, - local_map: &IndexVec>, - #[allow(rustc::default_hash_types)] value_labels_ranges: &std::collections::HashMap< - ValueLabel, - Vec, - >, - place: Place<'tcx>, -) -> AttributeValue { - assert!(place.projection.is_empty()); // FIXME implement them - - match local_map[place.local].inner() { - CPlaceInner::Var(_local, var) => { - let value_label = cranelift_codegen::ir::ValueLabel::new(var.index()); - if let Some(value_loc_ranges) = value_labels_ranges.get(&value_label) { - let loc_list = LocationList( - value_loc_ranges - .iter() - .map(|value_loc_range| Location::StartEnd { - begin: Address::Symbol { - symbol, - addend: i64::from(value_loc_range.start), - }, - end: Address::Symbol { symbol, addend: i64::from(value_loc_range.end) }, - data: translate_loc(isa, value_loc_range.loc).unwrap(), - }) - .collect(), - ); - let loc_list_id = debug_context.dwarf.unit.locations.add(loc_list); - - AttributeValue::LocationListRef(loc_list_id) - } else { - // FIXME set value labels for unused locals - - AttributeValue::Exprloc(Expression::new()) - } - } - CPlaceInner::VarPair(_, _, _) => { - // FIXME implement this - - AttributeValue::Exprloc(Expression::new()) - } - CPlaceInner::VarLane(_, _, _) => { - // FIXME implement this - - AttributeValue::Exprloc(Expression::new()) - } - CPlaceInner::Addr(_, _) => { - // FIXME implement this (used by arguments and returns) - - AttributeValue::Exprloc(Expression::new()) - - // For PointerBase::Stack: - //AttributeValue::Exprloc(translate_loc(ValueLoc::Stack(*stack_slot)).unwrap()) - } - } -} - -// Adapted from https://github.com/CraneStation/wasmtime/blob/5a1845b4caf7a5dba8eda1fef05213a532ed4259/crates/debug/src/transform/expression.rs#L59-L137 -fn translate_loc(isa: &dyn TargetIsa, loc: LabelValueLoc) -> Option { - match loc { - LabelValueLoc::Reg(reg) => { - let machine_reg = isa.map_regalloc_reg_to_dwarf(reg).unwrap(); - let mut expr = Expression::new(); - expr.op_reg(gimli::Register(machine_reg)); - Some(expr) - } - LabelValueLoc::SPOffset(offset) => { - let mut expr = Expression::new(); - expr.op_breg(X86_64::RSP, offset); - Some(expr) - } } } From e5493a5ea260a4f112a081c4bc488d85049ca36c Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 18 Aug 2022 15:19:29 +0000 Subject: [PATCH 07/15] Remove Instance param of DebugContext::define_function --- src/base.rs | 8 ++++---- src/debuginfo/mod.rs | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/base.rs b/src/base.rs index fe04567af..b35f02e2d 100644 --- a/src/base.rs +++ b/src/base.rs @@ -14,11 +14,11 @@ use crate::prelude::*; use crate::pretty_clif::CommentWriter; struct CodegenedFunction<'tcx> { - instance: Instance<'tcx>, symbol_name: SymbolName<'tcx>, func_id: FuncId, func: Function, clif_comments: CommentWriter, + function_span: Span, source_info_set: IndexSet, } @@ -108,8 +108,8 @@ fn codegen_fn<'tcx>( tcx.sess.time("codegen clif ir", || codegen_fn_body(&mut fx, start_block)); // Recover all necessary data from fx, before accessing func will prevent future access to it. - let instance = fx.instance; let clif_comments = fx.clif_comments; + let function_span = fx.mir.span; let source_info_set = fx.source_info_set; fx.constants_cx.finalize(fx.tcx, &mut *fx.module); @@ -128,7 +128,7 @@ fn codegen_fn<'tcx>( // Verify function verify_func(tcx, &clif_comments, &func); - CodegenedFunction { instance, symbol_name, func_id, func, clif_comments, source_info_set } + CodegenedFunction { symbol_name, func_id, func, clif_comments, function_span, source_info_set } } fn compile_fn<'tcx>( @@ -214,10 +214,10 @@ fn compile_fn<'tcx>( cx.profiler.verbose_generic_activity("generate debug info").run(|| { if let Some(debug_context) = debug_context { debug_context.define_function( - codegened_func.instance, codegened_func.func_id, codegened_func.symbol_name.name, context, + codegened_func.function_span, &codegened_func.source_info_set, ); } diff --git a/src/debuginfo/mod.rs b/src/debuginfo/mod.rs index 7a330ff7d..9a0787508 100644 --- a/src/debuginfo/mod.rs +++ b/src/debuginfo/mod.rs @@ -97,14 +97,13 @@ impl<'tcx> DebugContext<'tcx> { pub(crate) fn define_function( &mut self, - instance: Instance<'tcx>, func_id: FuncId, name: &str, context: &Context, + function_span: Span, source_info_set: &indexmap::IndexSet, ) { let symbol = func_id.as_u32() as usize; - let mir = self.tcx.instance_mir(instance.def); // FIXME: add to appropriate scope instead of root let scope = self.dwarf.unit.root(); @@ -116,7 +115,8 @@ impl<'tcx> DebugContext<'tcx> { entry.set(gimli::DW_AT_name, AttributeValue::StringRef(name_id)); entry.set(gimli::DW_AT_linkage_name, AttributeValue::StringRef(name_id)); - let end = self.create_debug_lines(symbol, entry_id, context, mir.span, source_info_set); + let end = + self.create_debug_lines(symbol, entry_id, context, function_span, source_info_set); self.unit_range_list.0.push(Range::StartLength { begin: Address::Symbol { symbol, addend: 0 }, From 259b21fd46b9ccfd084bce3ec8043543ca9ea61a Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 18 Aug 2022 15:25:26 +0000 Subject: [PATCH 08/15] Remove TyCtxt from DebugContext And explicitly thread it through everwhere it is needed. --- src/base.rs | 10 ++++++---- src/common.rs | 2 +- src/debuginfo/emit.rs | 2 +- src/debuginfo/line_info.rs | 10 +++++----- src/debuginfo/mod.rs | 13 ++++++------- src/driver/aot.rs | 2 +- src/driver/jit.rs | 8 ++++---- src/lib.rs | 8 ++++---- 8 files changed, 28 insertions(+), 27 deletions(-) diff --git a/src/base.rs b/src/base.rs index b35f02e2d..b2026f30d 100644 --- a/src/base.rs +++ b/src/base.rs @@ -24,7 +24,7 @@ struct CodegenedFunction<'tcx> { pub(crate) fn codegen_and_compile_fn<'tcx>( tcx: TyCtxt<'tcx>, - cx: &mut crate::CodegenCx<'tcx>, + cx: &mut crate::CodegenCx, cached_context: &mut Context, module: &mut dyn Module, instance: Instance<'tcx>, @@ -35,12 +35,12 @@ pub(crate) fn codegen_and_compile_fn<'tcx>( let cached_func = std::mem::replace(&mut cached_context.func, Function::new()); let codegened_func = codegen_fn(tcx, cx, cached_func, module, instance); - compile_fn(cx, cached_context, module, codegened_func); + compile_fn(tcx, cx, cached_context, module, codegened_func); } fn codegen_fn<'tcx>( tcx: TyCtxt<'tcx>, - cx: &mut crate::CodegenCx<'tcx>, + cx: &mut crate::CodegenCx, cached_func: Function, module: &mut dyn Module, instance: Instance<'tcx>, @@ -132,7 +132,8 @@ fn codegen_fn<'tcx>( } fn compile_fn<'tcx>( - cx: &mut crate::CodegenCx<'tcx>, + tcx: TyCtxt<'tcx>, + cx: &mut crate::CodegenCx, cached_context: &mut Context, module: &mut dyn Module, codegened_func: CodegenedFunction<'tcx>, @@ -214,6 +215,7 @@ fn compile_fn<'tcx>( cx.profiler.verbose_generic_activity("generate debug info").run(|| { if let Some(debug_context) = debug_context { debug_context.define_function( + tcx, codegened_func.func_id, codegened_func.symbol_name.name, context, diff --git a/src/common.rs b/src/common.rs index fc4953cea..1adb64da8 100644 --- a/src/common.rs +++ b/src/common.rs @@ -232,7 +232,7 @@ pub(crate) fn type_sign(ty: Ty<'_>) -> bool { } pub(crate) struct FunctionCx<'m, 'clif, 'tcx: 'm> { - pub(crate) cx: &'clif mut crate::CodegenCx<'tcx>, + pub(crate) cx: &'clif mut crate::CodegenCx, pub(crate) module: &'m mut dyn Module, pub(crate) tcx: TyCtxt<'tcx>, pub(crate) target_config: TargetFrontendConfig, // Cached from module diff --git a/src/debuginfo/emit.rs b/src/debuginfo/emit.rs index 589910ede..9583cd2ec 100644 --- a/src/debuginfo/emit.rs +++ b/src/debuginfo/emit.rs @@ -9,7 +9,7 @@ use gimli::{RunTimeEndian, SectionId}; use super::object::WriteDebugInfo; use super::DebugContext; -impl DebugContext<'_> { +impl DebugContext { pub(crate) fn emit(&mut self, product: &mut ObjectProduct) { let unit_range_list_id = self.dwarf.unit.ranges.add(self.unit_range_list.clone()); let root = self.dwarf.unit.root(); diff --git a/src/debuginfo/line_info.rs b/src/debuginfo/line_info.rs index bbcb95913..de402a4c7 100644 --- a/src/debuginfo/line_info.rs +++ b/src/debuginfo/line_info.rs @@ -96,9 +96,9 @@ fn line_program_add_file( } } -impl<'tcx> DebugContext<'tcx> { - pub(super) fn emit_location(&mut self, entry_id: UnitEntryId, span: Span) { - let loc = self.tcx.sess.source_map().lookup_char_pos(span.lo()); +impl DebugContext { + fn emit_location(&mut self, tcx: TyCtxt<'_>, entry_id: UnitEntryId, span: Span) { + let loc = tcx.sess.source_map().lookup_char_pos(span.lo()); let file_id = line_program_add_file( &mut self.dwarf.unit.line_program, @@ -115,13 +115,13 @@ impl<'tcx> DebugContext<'tcx> { pub(super) fn create_debug_lines( &mut self, + tcx: TyCtxt<'_>, symbol: usize, entry_id: UnitEntryId, context: &Context, function_span: Span, source_info_set: &indexmap::IndexSet, ) -> CodeOffset { - let tcx = self.tcx; let line_program = &mut self.dwarf.unit.line_program; let line_strings = &mut self.dwarf.line_strings; @@ -211,7 +211,7 @@ impl<'tcx> DebugContext<'tcx> { ); entry.set(gimli::DW_AT_high_pc, AttributeValue::Udata(u64::from(func_end))); - self.emit_location(entry_id, function_span); + self.emit_location(tcx, entry_id, function_span); func_end } diff --git a/src/debuginfo/mod.rs b/src/debuginfo/mod.rs index 9a0787508..3e42905c8 100644 --- a/src/debuginfo/mod.rs +++ b/src/debuginfo/mod.rs @@ -16,17 +16,15 @@ use gimli::{Encoding, Format, LineEncoding, RunTimeEndian}; pub(crate) use emit::{DebugReloc, DebugRelocName}; pub(crate) use unwind::UnwindContext; -pub(crate) struct DebugContext<'tcx> { - tcx: TyCtxt<'tcx>, - +pub(crate) struct DebugContext { endian: RunTimeEndian, dwarf: DwarfUnit, unit_range_list: RangeList, } -impl<'tcx> DebugContext<'tcx> { - pub(crate) fn new(tcx: TyCtxt<'tcx>, isa: &dyn TargetIsa) -> Self { +impl DebugContext { + pub(crate) fn new(tcx: TyCtxt<'_>, isa: &dyn TargetIsa) -> Self { let encoding = Encoding { format: Format::Dwarf32, // FIXME this should be configurable @@ -92,11 +90,12 @@ impl<'tcx> DebugContext<'tcx> { root.set(gimli::DW_AT_low_pc, AttributeValue::Address(Address::Constant(0))); } - DebugContext { tcx, endian, dwarf, unit_range_list: RangeList(Vec::new()) } + DebugContext { endian, dwarf, unit_range_list: RangeList(Vec::new()) } } pub(crate) fn define_function( &mut self, + tcx: TyCtxt<'_>, func_id: FuncId, name: &str, context: &Context, @@ -116,7 +115,7 @@ impl<'tcx> DebugContext<'tcx> { entry.set(gimli::DW_AT_linkage_name, AttributeValue::StringRef(name_id)); let end = - self.create_debug_lines(symbol, entry_id, context, function_span, source_info_set); + self.create_debug_lines(tcx, symbol, entry_id, context, function_span, source_info_set); self.unit_range_list.0.push(Range::StartLength { begin: Address::Symbol { symbol, addend: 0 }, diff --git a/src/driver/aot.rs b/src/driver/aot.rs index b2ad3cc4c..971e5eeea 100644 --- a/src/driver/aot.rs +++ b/src/driver/aot.rs @@ -120,7 +120,7 @@ fn emit_cgu( prof: &SelfProfilerRef, name: String, module: ObjectModule, - debug: Option>, + debug: Option, unwind_context: UnwindContext, global_asm_object_file: Option, ) -> Result { diff --git a/src/driver/jit.rs b/src/driver/jit.rs index 00e726344..0e77e4004 100644 --- a/src/driver/jit.rs +++ b/src/driver/jit.rs @@ -61,11 +61,11 @@ impl UnsafeMessage { } } -fn create_jit_module<'tcx>( - tcx: TyCtxt<'tcx>, +fn create_jit_module( + tcx: TyCtxt<'_>, backend_config: &BackendConfig, hotswap: bool, -) -> (JITModule, CodegenCx<'tcx>) { +) -> (JITModule, CodegenCx) { let crate_info = CrateInfo::new(tcx, "dummy_target_cpu".to_string()); let imported_symbols = load_imported_symbols_for_jit(tcx.sess, crate_info); @@ -353,7 +353,7 @@ fn load_imported_symbols_for_jit( fn codegen_shim<'tcx>( tcx: TyCtxt<'tcx>, - cx: &mut CodegenCx<'tcx>, + cx: &mut CodegenCx, cached_context: &mut Context, module: &mut JITModule, inst: Instance<'tcx>, diff --git a/src/lib.rs b/src/lib.rs index f4ffbbcf8..40dab5852 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -122,20 +122,20 @@ impl String> Drop for PrintOnPanic { /// The codegen context holds any information shared between the codegen of individual functions /// inside a single codegen unit with the exception of the Cranelift [`Module`](cranelift_module::Module). -struct CodegenCx<'tcx> { +struct CodegenCx { profiler: SelfProfilerRef, output_filenames: Arc, should_write_ir: bool, global_asm: String, inline_asm_index: Cell, - debug_context: Option>, + debug_context: Option, unwind_context: UnwindContext, cgu_name: Symbol, } -impl<'tcx> CodegenCx<'tcx> { +impl CodegenCx { fn new( - tcx: TyCtxt<'tcx>, + tcx: TyCtxt<'_>, backend_config: BackendConfig, isa: &dyn TargetIsa, debug_info: bool, From 312563f3c46c52e60afd499788b4950248e2c8d4 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 18 Aug 2022 15:57:21 +0000 Subject: [PATCH 09/15] Use walk_chain for function span too This is a correctness fix --- src/debuginfo/line_info.rs | 85 ++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 45 deletions(-) diff --git a/src/debuginfo/line_info.rs b/src/debuginfo/line_info.rs index de402a4c7..eb1365167 100644 --- a/src/debuginfo/line_info.rs +++ b/src/debuginfo/line_info.rs @@ -5,6 +5,7 @@ use std::path::{Component, Path}; use crate::prelude::*; +use rustc_data_structures::sync::Lrc; use rustc_span::{ FileName, Pos, SourceFile, SourceFileAndLine, SourceFileHash, SourceFileHashAlgorithm, }; @@ -47,9 +48,33 @@ fn osstr_as_utf8_bytes(path: &OsStr) -> &[u8] { } } -pub(crate) const MD5_LEN: usize = 16; +fn get_span_loc(tcx: TyCtxt<'_>, function_span: Span, span: Span) -> (Lrc, u64, u64) { + // Based on https://github.com/rust-lang/rust/blob/e369d87b015a84653343032833d65d0545fd3f26/src/librustc_codegen_ssa/mir/mod.rs#L116-L131 + // In order to have a good line stepping behavior in debugger, we overwrite debug + // locations of macro expansions with that of the outermost expansion site + // (unless the crate is being compiled with `-Z debug-macros`). + let span = if !span.from_expansion() || tcx.sess.opts.unstable_opts.debug_macros { + span + } else { + // Walk up the macro expansion chain until we reach a non-expanded span. + // We also stop at the function body level because no line stepping can occur + // at the level above that. + rustc_span::hygiene::walk_chain(span, function_span.ctxt()) + }; + + match tcx.sess.source_map().lookup_line(span.lo()) { + Ok(SourceFileAndLine { sf: file, line }) => { + let line_pos = file.line_begin_pos(span.lo()); + + (file, u64::try_from(line).unwrap() + 1, u64::from((span.lo() - line_pos).to_u32()) + 1) + } + Err(file) => (file, 0, 0), + } +} + +const MD5_LEN: usize = 16; -pub(crate) fn make_file_info(hash: SourceFileHash) -> Option { +fn make_file_info(hash: SourceFileHash) -> Option { if hash.kind == SourceFileHashAlgorithm::Md5 { let mut buf = [0u8; MD5_LEN]; buf.copy_from_slice(hash.hash_bytes()); @@ -97,22 +122,6 @@ fn line_program_add_file( } impl DebugContext { - fn emit_location(&mut self, tcx: TyCtxt<'_>, entry_id: UnitEntryId, span: Span) { - let loc = tcx.sess.source_map().lookup_char_pos(span.lo()); - - let file_id = line_program_add_file( - &mut self.dwarf.unit.line_program, - &mut self.dwarf.line_strings, - &loc.file, - ); - - let entry = self.dwarf.unit.get_mut(entry_id); - - entry.set(gimli::DW_AT_decl_file, AttributeValue::FileIndex(Some(file_id))); - entry.set(gimli::DW_AT_decl_line, AttributeValue::Udata(loc.line as u64)); - entry.set(gimli::DW_AT_decl_column, AttributeValue::Udata(loc.col.to_usize() as u64)); - } - pub(super) fn create_debug_lines( &mut self, tcx: TyCtxt<'_>, @@ -136,31 +145,7 @@ impl DebugContext { } last_span = Some(span); - // Based on https://github.com/rust-lang/rust/blob/e369d87b015a84653343032833d65d0545fd3f26/src/librustc_codegen_ssa/mir/mod.rs#L116-L131 - // In order to have a good line stepping behavior in debugger, we overwrite debug - // locations of macro expansions with that of the outermost expansion site - // (unless the crate is being compiled with `-Z debug-macros`). - let span = if !span.from_expansion() || tcx.sess.opts.unstable_opts.debug_macros { - span - } else { - // Walk up the macro expansion chain until we reach a non-expanded span. - // We also stop at the function body level because no line stepping can occur - // at the level above that. - rustc_span::hygiene::walk_chain(span, function_span.ctxt()) - }; - - let (file, line, col) = match tcx.sess.source_map().lookup_line(span.lo()) { - Ok(SourceFileAndLine { sf: file, line }) => { - let line_pos = file.line_begin_pos(span.lo()); - - ( - file, - u64::try_from(line).unwrap() + 1, - u64::from((span.lo() - line_pos).to_u32()) + 1, - ) - } - Err(file) => (file, 0, 0), - }; + let (file, line, col) = get_span_loc(tcx, function_span, span); // line_program_add_file is very slow. // Optimize for the common case of the current file not being changed. @@ -204,14 +189,24 @@ impl DebugContext { assert_ne!(func_end, 0); + let (function_file, function_line, function_col) = + get_span_loc(tcx, function_span, function_span); + + let function_file_id = line_program_add_file( + &mut self.dwarf.unit.line_program, + &mut self.dwarf.line_strings, + &function_file, + ); + let entry = self.dwarf.unit.get_mut(entry_id); entry.set( gimli::DW_AT_low_pc, AttributeValue::Address(Address::Symbol { symbol, addend: 0 }), ); entry.set(gimli::DW_AT_high_pc, AttributeValue::Udata(u64::from(func_end))); - - self.emit_location(tcx, entry_id, function_span); + entry.set(gimli::DW_AT_decl_file, AttributeValue::FileIndex(Some(function_file_id))); + entry.set(gimli::DW_AT_decl_line, AttributeValue::Udata(function_line)); + entry.set(gimli::DW_AT_decl_column, AttributeValue::Udata(function_col)); func_end } From dbf545730849375ce43f593432f2bff5eddd3eec Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 18 Aug 2022 17:11:41 +0000 Subject: [PATCH 10/15] Introduce FunctionDebugContext This will make it easier to move TyCtxt requiring operations before clif ir compilation. --- src/base.rs | 4 ++-- src/debuginfo/mod.rs | 50 ++++++++++++++++++++++++++++++-------------- 2 files changed, 36 insertions(+), 18 deletions(-) diff --git a/src/base.rs b/src/base.rs index b2026f30d..73af2ea33 100644 --- a/src/base.rs +++ b/src/base.rs @@ -214,10 +214,10 @@ fn compile_fn<'tcx>( let unwind_context = &mut cx.unwind_context; cx.profiler.verbose_generic_activity("generate debug info").run(|| { if let Some(debug_context) = debug_context { - debug_context.define_function( + debug_context.define_function(codegened_func.symbol_name.name).finalize( + debug_context, tcx, codegened_func.func_id, - codegened_func.symbol_name.name, context, codegened_func.function_span, &codegened_func.source_info_set, diff --git a/src/debuginfo/mod.rs b/src/debuginfo/mod.rs index 3e42905c8..072680700 100644 --- a/src/debuginfo/mod.rs +++ b/src/debuginfo/mod.rs @@ -10,7 +10,9 @@ use crate::prelude::*; use cranelift_codegen::ir::Endianness; use cranelift_codegen::isa::TargetIsa; -use gimli::write::{Address, AttributeValue, DwarfUnit, LineProgram, LineString, Range, RangeList}; +use gimli::write::{ + Address, AttributeValue, DwarfUnit, LineProgram, LineString, Range, RangeList, UnitEntryId, +}; use gimli::{Encoding, Format, LineEncoding, RunTimeEndian}; pub(crate) use emit::{DebugReloc, DebugRelocName}; @@ -23,6 +25,10 @@ pub(crate) struct DebugContext { unit_range_list: RangeList, } +pub(crate) struct FunctionDebugContext { + entry_id: UnitEntryId, +} + impl DebugContext { pub(crate) fn new(tcx: TyCtxt<'_>, isa: &dyn TargetIsa) -> Self { let encoding = Encoding { @@ -93,17 +99,7 @@ impl DebugContext { DebugContext { endian, dwarf, unit_range_list: RangeList(Vec::new()) } } - pub(crate) fn define_function( - &mut self, - tcx: TyCtxt<'_>, - func_id: FuncId, - name: &str, - context: &Context, - function_span: Span, - source_info_set: &indexmap::IndexSet, - ) { - let symbol = func_id.as_u32() as usize; - + pub(crate) fn define_function(&mut self, name: &str) -> FunctionDebugContext { // FIXME: add to appropriate scope instead of root let scope = self.dwarf.unit.root(); @@ -114,15 +110,37 @@ impl DebugContext { entry.set(gimli::DW_AT_name, AttributeValue::StringRef(name_id)); entry.set(gimli::DW_AT_linkage_name, AttributeValue::StringRef(name_id)); - let end = - self.create_debug_lines(tcx, symbol, entry_id, context, function_span, source_info_set); + FunctionDebugContext { entry_id } + } +} + +impl FunctionDebugContext { + pub(crate) fn finalize( + self, + debug_context: &mut DebugContext, + tcx: TyCtxt<'_>, + func_id: FuncId, + context: &Context, + function_span: Span, + source_info_set: &indexmap::IndexSet, + ) { + let symbol = func_id.as_u32() as usize; + + let end = debug_context.create_debug_lines( + tcx, + symbol, + self.entry_id, + context, + function_span, + source_info_set, + ); - self.unit_range_list.0.push(Range::StartLength { + debug_context.unit_range_list.0.push(Range::StartLength { begin: Address::Symbol { symbol, addend: 0 }, length: u64::from(end), }); - let func_entry = self.dwarf.unit.get_mut(entry_id); + let func_entry = debug_context.dwarf.unit.get_mut(self.entry_id); // Gdb requires both DW_AT_low_pc and DW_AT_high_pc. Otherwise the DW_TAG_subprogram is skipped. func_entry.set( gimli::DW_AT_low_pc, From 01be0ddacfa84559947d024c6060a99c63c48e29 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 18 Aug 2022 17:17:33 +0000 Subject: [PATCH 11/15] Move FunctionDebugContext creation to codegen_fn --- src/base.rs | 22 ++++++++++++++++++++-- src/common.rs | 2 ++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/base.rs b/src/base.rs index 73af2ea33..34dbf96be 100644 --- a/src/base.rs +++ b/src/base.rs @@ -10,6 +10,7 @@ use rustc_middle::ty::SymbolName; use indexmap::IndexSet; use crate::constant::ConstantCx; +use crate::debuginfo::FunctionDebugContext; use crate::prelude::*; use crate::pretty_clif::CommentWriter; @@ -18,6 +19,7 @@ struct CodegenedFunction<'tcx> { func_id: FuncId, func: Function, clif_comments: CommentWriter, + func_debug_cx: Option, function_span: Span, source_info_set: IndexSet, } @@ -82,6 +84,12 @@ fn codegen_fn<'tcx>( let pointer_type = target_config.pointer_type(); let clif_comments = crate::pretty_clif::CommentWriter::new(tcx, instance); + let func_debug_cx = if let Some(debug_context) = &mut cx.debug_context { + Some(debug_context.define_function(symbol_name.name)) + } else { + None + }; + let mut fx = FunctionCx { cx, module, @@ -89,6 +97,7 @@ fn codegen_fn<'tcx>( target_config, pointer_type, constants_cx: ConstantCx::new(), + func_debug_cx, instance, symbol_name, @@ -109,6 +118,7 @@ fn codegen_fn<'tcx>( // Recover all necessary data from fx, before accessing func will prevent future access to it. let clif_comments = fx.clif_comments; + let func_debug_cx = fx.func_debug_cx; let function_span = fx.mir.span; let source_info_set = fx.source_info_set; @@ -128,7 +138,15 @@ fn codegen_fn<'tcx>( // Verify function verify_func(tcx, &clif_comments, &func); - CodegenedFunction { symbol_name, func_id, func, clif_comments, function_span, source_info_set } + CodegenedFunction { + symbol_name, + func_id, + func, + clif_comments, + func_debug_cx, + function_span, + source_info_set, + } } fn compile_fn<'tcx>( @@ -214,7 +232,7 @@ fn compile_fn<'tcx>( let unwind_context = &mut cx.unwind_context; cx.profiler.verbose_generic_activity("generate debug info").run(|| { if let Some(debug_context) = debug_context { - debug_context.define_function(codegened_func.symbol_name.name).finalize( + codegened_func.func_debug_cx.unwrap().finalize( debug_context, tcx, codegened_func.func_id, diff --git a/src/common.rs b/src/common.rs index 1adb64da8..655ceacf7 100644 --- a/src/common.rs +++ b/src/common.rs @@ -9,6 +9,7 @@ use rustc_target::abi::{Integer, Primitive}; use rustc_target::spec::{HasTargetSpec, Target}; use crate::constant::ConstantCx; +use crate::debuginfo::FunctionDebugContext; use crate::prelude::*; pub(crate) fn pointer_ty(tcx: TyCtxt<'_>) -> types::Type { @@ -238,6 +239,7 @@ pub(crate) struct FunctionCx<'m, 'clif, 'tcx: 'm> { pub(crate) target_config: TargetFrontendConfig, // Cached from module pub(crate) pointer_type: Type, // Cached from module pub(crate) constants_cx: ConstantCx, + pub(crate) func_debug_cx: Option, pub(crate) instance: Instance<'tcx>, pub(crate) symbol_name: SymbolName<'tcx>, From 1e57774011ab634a04c0efd3bf2e956ee8997245 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 18 Aug 2022 17:23:36 +0000 Subject: [PATCH 12/15] Move set_function_span earlier --- src/base.rs | 2 +- src/debuginfo/line_info.rs | 44 ++++++++++++++++++++++---------------- src/debuginfo/mod.rs | 19 +++++++++++----- 3 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/base.rs b/src/base.rs index 34dbf96be..8440a0333 100644 --- a/src/base.rs +++ b/src/base.rs @@ -85,7 +85,7 @@ fn codegen_fn<'tcx>( let clif_comments = crate::pretty_clif::CommentWriter::new(tcx, instance); let func_debug_cx = if let Some(debug_context) = &mut cx.debug_context { - Some(debug_context.define_function(symbol_name.name)) + Some(debug_context.define_function(tcx, symbol_name.name, mir.span)) } else { None }; diff --git a/src/debuginfo/line_info.rs b/src/debuginfo/line_info.rs index eb1365167..5a13b9681 100644 --- a/src/debuginfo/line_info.rs +++ b/src/debuginfo/line_info.rs @@ -3,6 +3,7 @@ use std::ffi::OsStr; use std::path::{Component, Path}; +use crate::debuginfo::FunctionDebugContext; use crate::prelude::*; use rustc_data_structures::sync::Lrc; @@ -15,7 +16,6 @@ use cranelift_codegen::MachSrcLoc; use gimli::write::{ Address, AttributeValue, FileId, FileInfo, LineProgram, LineString, LineStringTable, - UnitEntryId, }; // OPTIMIZATION: It is cheaper to do this in one pass than using `.parent()` and `.file_name()`. @@ -121,19 +121,39 @@ fn line_program_add_file( } } -impl DebugContext { +impl FunctionDebugContext { + pub(super) fn set_function_span( + &mut self, + debug_context: &mut DebugContext, + tcx: TyCtxt<'_>, + span: Span, + ) { + let (file, line, column) = get_span_loc(tcx, span, span); + + let file_id = line_program_add_file( + &mut debug_context.dwarf.unit.line_program, + &mut debug_context.dwarf.line_strings, + &file, + ); + + let entry = debug_context.dwarf.unit.get_mut(self.entry_id); + entry.set(gimli::DW_AT_decl_file, AttributeValue::FileIndex(Some(file_id))); + entry.set(gimli::DW_AT_decl_line, AttributeValue::Udata(line)); + entry.set(gimli::DW_AT_decl_column, AttributeValue::Udata(column)); + } + pub(super) fn create_debug_lines( &mut self, + debug_context: &mut DebugContext, tcx: TyCtxt<'_>, symbol: usize, - entry_id: UnitEntryId, context: &Context, function_span: Span, source_info_set: &indexmap::IndexSet, ) -> CodeOffset { - let line_program = &mut self.dwarf.unit.line_program; + let line_program = &mut debug_context.dwarf.unit.line_program; - let line_strings = &mut self.dwarf.line_strings; + let line_strings = &mut debug_context.dwarf.line_strings; let mut last_span = None; let mut last_file = None; let mut create_row_for_span = |line_program: &mut LineProgram, span: Span| { @@ -189,24 +209,12 @@ impl DebugContext { assert_ne!(func_end, 0); - let (function_file, function_line, function_col) = - get_span_loc(tcx, function_span, function_span); - - let function_file_id = line_program_add_file( - &mut self.dwarf.unit.line_program, - &mut self.dwarf.line_strings, - &function_file, - ); - - let entry = self.dwarf.unit.get_mut(entry_id); + let entry = debug_context.dwarf.unit.get_mut(self.entry_id); entry.set( gimli::DW_AT_low_pc, AttributeValue::Address(Address::Symbol { symbol, addend: 0 }), ); entry.set(gimli::DW_AT_high_pc, AttributeValue::Udata(u64::from(func_end))); - entry.set(gimli::DW_AT_decl_file, AttributeValue::FileIndex(Some(function_file_id))); - entry.set(gimli::DW_AT_decl_line, AttributeValue::Udata(function_line)); - entry.set(gimli::DW_AT_decl_column, AttributeValue::Udata(function_col)); func_end } diff --git a/src/debuginfo/mod.rs b/src/debuginfo/mod.rs index 072680700..169b7d1ef 100644 --- a/src/debuginfo/mod.rs +++ b/src/debuginfo/mod.rs @@ -99,7 +99,12 @@ impl DebugContext { DebugContext { endian, dwarf, unit_range_list: RangeList(Vec::new()) } } - pub(crate) fn define_function(&mut self, name: &str) -> FunctionDebugContext { + pub(crate) fn define_function( + &mut self, + tcx: TyCtxt<'_>, + name: &str, + function_span: Span, + ) -> FunctionDebugContext { // FIXME: add to appropriate scope instead of root let scope = self.dwarf.unit.root(); @@ -110,13 +115,17 @@ impl DebugContext { entry.set(gimli::DW_AT_name, AttributeValue::StringRef(name_id)); entry.set(gimli::DW_AT_linkage_name, AttributeValue::StringRef(name_id)); - FunctionDebugContext { entry_id } + let mut function_debug_context = FunctionDebugContext { entry_id }; + + function_debug_context.set_function_span(self, tcx, function_span); + + function_debug_context } } impl FunctionDebugContext { pub(crate) fn finalize( - self, + mut self, debug_context: &mut DebugContext, tcx: TyCtxt<'_>, func_id: FuncId, @@ -126,10 +135,10 @@ impl FunctionDebugContext { ) { let symbol = func_id.as_u32() as usize; - let end = debug_context.create_debug_lines( + let end = self.create_debug_lines( + debug_context, tcx, symbol, - self.entry_id, context, function_span, source_info_set, From 0534a555ccb1a62c2f379bbd2e8cd323e6652e31 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 18 Aug 2022 17:48:22 +0000 Subject: [PATCH 13/15] Make line_program_add_file a DebugContext method --- src/debuginfo/line_info.rs | 105 ++++++++++++++++++------------------- 1 file changed, 52 insertions(+), 53 deletions(-) diff --git a/src/debuginfo/line_info.rs b/src/debuginfo/line_info.rs index 5a13b9681..4fdaaf4ff 100644 --- a/src/debuginfo/line_info.rs +++ b/src/debuginfo/line_info.rs @@ -84,39 +84,41 @@ fn make_file_info(hash: SourceFileHash) -> Option { } } -fn line_program_add_file( - line_program: &mut LineProgram, - line_strings: &mut LineStringTable, - file: &SourceFile, -) -> FileId { - match &file.name { - FileName::Real(path) => { - let (dir_path, file_name) = split_path_dir_and_file(path.remapped_path_if_available()); - let dir_name = osstr_as_utf8_bytes(dir_path.as_os_str()); - let file_name = osstr_as_utf8_bytes(file_name); - - let dir_id = if !dir_name.is_empty() { - let dir_name = LineString::new(dir_name, line_program.encoding(), line_strings); - line_program.add_directory(dir_name) - } else { - line_program.default_directory() - }; - let file_name = LineString::new(file_name, line_program.encoding(), line_strings); - - let info = make_file_info(file.src_hash); - - line_program.file_has_md5 &= info.is_some(); - line_program.add_file(file_name, dir_id, info) - } - // FIXME give more appropriate file names - filename => { - let dir_id = line_program.default_directory(); - let dummy_file_name = LineString::new( - filename.prefer_remapped().to_string().into_bytes(), - line_program.encoding(), - line_strings, - ); - line_program.add_file(dummy_file_name, dir_id, None) +impl DebugContext { + pub(crate) fn add_source_file(&mut self, source_file: &SourceFile) -> FileId { + let line_program: &mut LineProgram = &mut self.dwarf.unit.line_program; + let line_strings: &mut LineStringTable = &mut self.dwarf.line_strings; + + match &source_file.name { + FileName::Real(path) => { + let (dir_path, file_name) = + split_path_dir_and_file(path.remapped_path_if_available()); + let dir_name = osstr_as_utf8_bytes(dir_path.as_os_str()); + let file_name = osstr_as_utf8_bytes(file_name); + + let dir_id = if !dir_name.is_empty() { + let dir_name = LineString::new(dir_name, line_program.encoding(), line_strings); + line_program.add_directory(dir_name) + } else { + line_program.default_directory() + }; + let file_name = LineString::new(file_name, line_program.encoding(), line_strings); + + let info = make_file_info(source_file.src_hash); + + line_program.file_has_md5 &= info.is_some(); + line_program.add_file(file_name, dir_id, info) + } + // FIXME give more appropriate file names + filename => { + let dir_id = line_program.default_directory(); + let dummy_file_name = LineString::new( + filename.prefer_remapped().to_string().into_bytes(), + line_program.encoding(), + line_strings, + ); + line_program.add_file(dummy_file_name, dir_id, None) + } } } } @@ -130,11 +132,7 @@ impl FunctionDebugContext { ) { let (file, line, column) = get_span_loc(tcx, span, span); - let file_id = line_program_add_file( - &mut debug_context.dwarf.unit.line_program, - &mut debug_context.dwarf.line_strings, - &file, - ); + let file_id = debug_context.add_source_file(&file); let entry = debug_context.dwarf.unit.get_mut(self.entry_id); entry.set(gimli::DW_AT_decl_file, AttributeValue::FileIndex(Some(file_id))); @@ -151,15 +149,12 @@ impl FunctionDebugContext { function_span: Span, source_info_set: &indexmap::IndexSet, ) -> CodeOffset { - let line_program = &mut debug_context.dwarf.unit.line_program; - - let line_strings = &mut debug_context.dwarf.line_strings; let mut last_span = None; let mut last_file = None; - let mut create_row_for_span = |line_program: &mut LineProgram, span: Span| { + let mut create_row_for_span = |debug_context: &mut DebugContext, span: Span| { if let Some(last_span) = last_span { if span == last_span { - line_program.generate_row(); + debug_context.dwarf.unit.line_program.generate_row(); return; } } @@ -177,33 +172,37 @@ impl FunctionDebugContext { true }; if current_file_changed { - let file_id = line_program_add_file(line_program, line_strings, &file); - line_program.row().file = file_id; + let file_id = debug_context.add_source_file(&file); + debug_context.dwarf.unit.line_program.row().file = file_id; last_file = Some(file); } - line_program.row().line = line; - line_program.row().column = col; - line_program.generate_row(); + debug_context.dwarf.unit.line_program.row().line = line; + debug_context.dwarf.unit.line_program.row().column = col; + debug_context.dwarf.unit.line_program.generate_row(); }; - line_program.begin_sequence(Some(Address::Symbol { symbol, addend: 0 })); + debug_context + .dwarf + .unit + .line_program + .begin_sequence(Some(Address::Symbol { symbol, addend: 0 })); let mut func_end = 0; let mcr = context.mach_compile_result.as_ref().unwrap(); for &MachSrcLoc { start, end, loc } in mcr.buffer.get_srclocs_sorted() { - line_program.row().address_offset = u64::from(start); + debug_context.dwarf.unit.line_program.row().address_offset = u64::from(start); if !loc.is_default() { let source_info = *source_info_set.get_index(loc.bits() as usize).unwrap(); - create_row_for_span(line_program, source_info.span); + create_row_for_span(debug_context, source_info.span); } else { - create_row_for_span(line_program, function_span); + create_row_for_span(debug_context, function_span); } func_end = end; } - line_program.end_sequence(u64::from(func_end)); + debug_context.dwarf.unit.line_program.end_sequence(u64::from(func_end)); let func_end = mcr.buffer.total_size(); From 6421427b74f87ae463a623edd684c34b50790bb2 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 18 Aug 2022 18:19:40 +0000 Subject: [PATCH 14/15] Move Span lowering from debuginfo finalization to codegen This removes the dependency on TyCtxt from the debuginfo finalization code. --- src/base.rs | 24 +------- src/common.rs | 37 +++++++++++- src/debuginfo/line_info.rs | 119 ++++++++++++++----------------------- src/debuginfo/mod.rs | 34 ++++++----- 4 files changed, 100 insertions(+), 114 deletions(-) diff --git a/src/base.rs b/src/base.rs index 8440a0333..5f6d9f374 100644 --- a/src/base.rs +++ b/src/base.rs @@ -7,8 +7,6 @@ use rustc_middle::ty::layout::FnAbiOf; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::SymbolName; -use indexmap::IndexSet; - use crate::constant::ConstantCx; use crate::debuginfo::FunctionDebugContext; use crate::prelude::*; @@ -20,8 +18,6 @@ struct CodegenedFunction<'tcx> { func: Function, clif_comments: CommentWriter, func_debug_cx: Option, - function_span: Span, - source_info_set: IndexSet, } pub(crate) fn codegen_and_compile_fn<'tcx>( @@ -37,7 +33,7 @@ pub(crate) fn codegen_and_compile_fn<'tcx>( let cached_func = std::mem::replace(&mut cached_context.func, Function::new()); let codegened_func = codegen_fn(tcx, cx, cached_func, module, instance); - compile_fn(tcx, cx, cached_context, module, codegened_func); + compile_fn(cx, cached_context, module, codegened_func); } fn codegen_fn<'tcx>( @@ -110,7 +106,7 @@ fn codegen_fn<'tcx>( caller_location: None, // set by `codegen_fn_prelude` clif_comments, - source_info_set: indexmap::IndexSet::new(), + last_source_file: None, next_ssa_var: 0, }; @@ -119,8 +115,6 @@ fn codegen_fn<'tcx>( // Recover all necessary data from fx, before accessing func will prevent future access to it. let clif_comments = fx.clif_comments; let func_debug_cx = fx.func_debug_cx; - let function_span = fx.mir.span; - let source_info_set = fx.source_info_set; fx.constants_cx.finalize(fx.tcx, &mut *fx.module); @@ -138,19 +132,10 @@ fn codegen_fn<'tcx>( // Verify function verify_func(tcx, &clif_comments, &func); - CodegenedFunction { - symbol_name, - func_id, - func, - clif_comments, - func_debug_cx, - function_span, - source_info_set, - } + CodegenedFunction { symbol_name, func_id, func, clif_comments, func_debug_cx } } fn compile_fn<'tcx>( - tcx: TyCtxt<'tcx>, cx: &mut crate::CodegenCx, cached_context: &mut Context, module: &mut dyn Module, @@ -234,11 +219,8 @@ fn compile_fn<'tcx>( if let Some(debug_context) = debug_context { codegened_func.func_debug_cx.unwrap().finalize( debug_context, - tcx, codegened_func.func_id, context, - codegened_func.function_span, - &codegened_func.source_info_set, ); } unwind_context.add_function(codegened_func.func_id, &context, isa); diff --git a/src/common.rs b/src/common.rs index 655ceacf7..4a80b79a9 100644 --- a/src/common.rs +++ b/src/common.rs @@ -1,9 +1,13 @@ use cranelift_codegen::isa::TargetFrontendConfig; +use gimli::write::FileId; + +use rustc_data_structures::sync::Lrc; use rustc_index::vec::IndexVec; use rustc_middle::ty::layout::{ FnAbiError, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOfHelpers, }; use rustc_middle::ty::SymbolName; +use rustc_span::SourceFile; use rustc_target::abi::call::FnAbi; use rustc_target::abi::{Integer, Primitive}; use rustc_target::spec::{HasTargetSpec, Target}; @@ -254,7 +258,11 @@ pub(crate) struct FunctionCx<'m, 'clif, 'tcx: 'm> { pub(crate) caller_location: Option>, pub(crate) clif_comments: crate::pretty_clif::CommentWriter, - pub(crate) source_info_set: indexmap::IndexSet, + + /// Last accessed source file and it's debuginfo file id. + /// + /// For optimization purposes only + pub(crate) last_source_file: Option<(Lrc, FileId)>, /// This should only be accessed by `CPlace::new_var`. pub(crate) next_ssa_var: u32, @@ -338,8 +346,31 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> { } pub(crate) fn set_debug_loc(&mut self, source_info: mir::SourceInfo) { - let (index, _) = self.source_info_set.insert_full(source_info); - self.bcx.set_srcloc(SourceLoc::new(index as u32)); + if let Some(debug_context) = &mut self.cx.debug_context { + let (file, line, column) = + DebugContext::get_span_loc(self.tcx, self.mir.span, source_info.span); + + // add_source_file is very slow. + // Optimize for the common case of the current file not being changed. + let mut cached_file_id = None; + if let Some((ref last_source_file, last_file_id)) = self.last_source_file { + // If the allocations are not equal, the files may still be equal, but that + // doesn't matter, as this is just an optimization. + if rustc_data_structures::sync::Lrc::ptr_eq(last_source_file, &file) { + cached_file_id = Some(last_file_id); + } + } + + let file_id = if let Some(file_id) = cached_file_id { + file_id + } else { + debug_context.add_source_file(&file) + }; + + let source_loc = + self.func_debug_cx.as_mut().unwrap().add_dbg_loc(file_id, line, column); + self.bcx.set_srcloc(source_loc); + } } // Note: must be kept in sync with get_caller_location from cg_ssa diff --git a/src/debuginfo/line_info.rs b/src/debuginfo/line_info.rs index 4fdaaf4ff..ff6a21eef 100644 --- a/src/debuginfo/line_info.rs +++ b/src/debuginfo/line_info.rs @@ -48,30 +48,6 @@ fn osstr_as_utf8_bytes(path: &OsStr) -> &[u8] { } } -fn get_span_loc(tcx: TyCtxt<'_>, function_span: Span, span: Span) -> (Lrc, u64, u64) { - // Based on https://github.com/rust-lang/rust/blob/e369d87b015a84653343032833d65d0545fd3f26/src/librustc_codegen_ssa/mir/mod.rs#L116-L131 - // In order to have a good line stepping behavior in debugger, we overwrite debug - // locations of macro expansions with that of the outermost expansion site - // (unless the crate is being compiled with `-Z debug-macros`). - let span = if !span.from_expansion() || tcx.sess.opts.unstable_opts.debug_macros { - span - } else { - // Walk up the macro expansion chain until we reach a non-expanded span. - // We also stop at the function body level because no line stepping can occur - // at the level above that. - rustc_span::hygiene::walk_chain(span, function_span.ctxt()) - }; - - match tcx.sess.source_map().lookup_line(span.lo()) { - Ok(SourceFileAndLine { sf: file, line }) => { - let line_pos = file.line_begin_pos(span.lo()); - - (file, u64::try_from(line).unwrap() + 1, u64::from((span.lo() - line_pos).to_u32()) + 1) - } - Err(file) => (file, 0, 0), - } -} - const MD5_LEN: usize = 16; fn make_file_info(hash: SourceFileHash) -> Option { @@ -85,6 +61,38 @@ fn make_file_info(hash: SourceFileHash) -> Option { } impl DebugContext { + pub(crate) fn get_span_loc( + tcx: TyCtxt<'_>, + function_span: Span, + span: Span, + ) -> (Lrc, u64, u64) { + // Based on https://github.com/rust-lang/rust/blob/e369d87b015a84653343032833d65d0545fd3f26/src/librustc_codegen_ssa/mir/mod.rs#L116-L131 + // In order to have a good line stepping behavior in debugger, we overwrite debug + // locations of macro expansions with that of the outermost expansion site + // (unless the crate is being compiled with `-Z debug-macros`). + let span = if !span.from_expansion() || tcx.sess.opts.unstable_opts.debug_macros { + span + } else { + // Walk up the macro expansion chain until we reach a non-expanded span. + // We also stop at the function body level because no line stepping can occur + // at the level above that. + rustc_span::hygiene::walk_chain(span, function_span.ctxt()) + }; + + match tcx.sess.source_map().lookup_line(span.lo()) { + Ok(SourceFileAndLine { sf: file, line }) => { + let line_pos = file.line_begin_pos(span.lo()); + + ( + file, + u64::try_from(line).unwrap() + 1, + u64::from((span.lo() - line_pos).to_u32()) + 1, + ) + } + Err(file) => (file, 0, 0), + } + } + pub(crate) fn add_source_file(&mut self, source_file: &SourceFile) -> FileId { let line_program: &mut LineProgram = &mut self.dwarf.unit.line_program; let line_strings: &mut LineStringTable = &mut self.dwarf.line_strings; @@ -124,63 +132,26 @@ impl DebugContext { } impl FunctionDebugContext { - pub(super) fn set_function_span( - &mut self, - debug_context: &mut DebugContext, - tcx: TyCtxt<'_>, - span: Span, - ) { - let (file, line, column) = get_span_loc(tcx, span, span); - - let file_id = debug_context.add_source_file(&file); - - let entry = debug_context.dwarf.unit.get_mut(self.entry_id); - entry.set(gimli::DW_AT_decl_file, AttributeValue::FileIndex(Some(file_id))); - entry.set(gimli::DW_AT_decl_line, AttributeValue::Udata(line)); - entry.set(gimli::DW_AT_decl_column, AttributeValue::Udata(column)); + pub(crate) fn add_dbg_loc(&mut self, file_id: FileId, line: u64, column: u64) -> SourceLoc { + let (index, _) = self.source_loc_set.insert_full((file_id, line, column)); + SourceLoc::new(u32::try_from(index).unwrap()) } pub(super) fn create_debug_lines( &mut self, debug_context: &mut DebugContext, - tcx: TyCtxt<'_>, symbol: usize, context: &Context, - function_span: Span, - source_info_set: &indexmap::IndexSet, ) -> CodeOffset { - let mut last_span = None; - let mut last_file = None; - let mut create_row_for_span = |debug_context: &mut DebugContext, span: Span| { - if let Some(last_span) = last_span { - if span == last_span { - debug_context.dwarf.unit.line_program.generate_row(); - return; - } - } - last_span = Some(span); - - let (file, line, col) = get_span_loc(tcx, function_span, span); + let create_row_for_span = + |debug_context: &mut DebugContext, source_loc: (FileId, u64, u64)| { + let (file_id, line, col) = source_loc; - // line_program_add_file is very slow. - // Optimize for the common case of the current file not being changed. - let current_file_changed = if let Some(last_file) = &last_file { - // If the allocations are not equal, then the files may still be equal, but that - // is not a problem, as this is just an optimization. - !rustc_data_structures::sync::Lrc::ptr_eq(last_file, &file) - } else { - true - }; - if current_file_changed { - let file_id = debug_context.add_source_file(&file); debug_context.dwarf.unit.line_program.row().file = file_id; - last_file = Some(file); - } - - debug_context.dwarf.unit.line_program.row().line = line; - debug_context.dwarf.unit.line_program.row().column = col; - debug_context.dwarf.unit.line_program.generate_row(); - }; + debug_context.dwarf.unit.line_program.row().line = line; + debug_context.dwarf.unit.line_program.row().column = col; + debug_context.dwarf.unit.line_program.generate_row(); + }; debug_context .dwarf @@ -194,10 +165,10 @@ impl FunctionDebugContext { for &MachSrcLoc { start, end, loc } in mcr.buffer.get_srclocs_sorted() { debug_context.dwarf.unit.line_program.row().address_offset = u64::from(start); if !loc.is_default() { - let source_info = *source_info_set.get_index(loc.bits() as usize).unwrap(); - create_row_for_span(debug_context, source_info.span); + let source_loc = *self.source_loc_set.get_index(loc.bits() as usize).unwrap(); + create_row_for_span(debug_context, source_loc); } else { - create_row_for_span(debug_context, function_span); + create_row_for_span(debug_context, self.function_source_loc); } func_end = end; } diff --git a/src/debuginfo/mod.rs b/src/debuginfo/mod.rs index 169b7d1ef..c55db2017 100644 --- a/src/debuginfo/mod.rs +++ b/src/debuginfo/mod.rs @@ -11,9 +11,11 @@ use cranelift_codegen::ir::Endianness; use cranelift_codegen::isa::TargetIsa; use gimli::write::{ - Address, AttributeValue, DwarfUnit, LineProgram, LineString, Range, RangeList, UnitEntryId, + Address, AttributeValue, DwarfUnit, FileId, LineProgram, LineString, Range, RangeList, + UnitEntryId, }; use gimli::{Encoding, Format, LineEncoding, RunTimeEndian}; +use indexmap::IndexSet; pub(crate) use emit::{DebugReloc, DebugRelocName}; pub(crate) use unwind::UnwindContext; @@ -27,6 +29,8 @@ pub(crate) struct DebugContext { pub(crate) struct FunctionDebugContext { entry_id: UnitEntryId, + function_source_loc: (FileId, u64, u64), + source_loc_set: indexmap::IndexSet<(FileId, u64, u64)>, } impl DebugContext { @@ -105,6 +109,10 @@ impl DebugContext { name: &str, function_span: Span, ) -> FunctionDebugContext { + let (file, line, column) = DebugContext::get_span_loc(tcx, function_span, function_span); + + let file_id = self.add_source_file(&file); + // FIXME: add to appropriate scope instead of root let scope = self.dwarf.unit.root(); @@ -115,11 +123,15 @@ impl DebugContext { entry.set(gimli::DW_AT_name, AttributeValue::StringRef(name_id)); entry.set(gimli::DW_AT_linkage_name, AttributeValue::StringRef(name_id)); - let mut function_debug_context = FunctionDebugContext { entry_id }; + entry.set(gimli::DW_AT_decl_file, AttributeValue::FileIndex(Some(file_id))); + entry.set(gimli::DW_AT_decl_line, AttributeValue::Udata(line)); + entry.set(gimli::DW_AT_decl_column, AttributeValue::Udata(column)); - function_debug_context.set_function_span(self, tcx, function_span); - - function_debug_context + FunctionDebugContext { + entry_id, + function_source_loc: (file_id, line, column), + source_loc_set: IndexSet::new(), + } } } @@ -127,22 +139,12 @@ impl FunctionDebugContext { pub(crate) fn finalize( mut self, debug_context: &mut DebugContext, - tcx: TyCtxt<'_>, func_id: FuncId, context: &Context, - function_span: Span, - source_info_set: &indexmap::IndexSet, ) { let symbol = func_id.as_u32() as usize; - let end = self.create_debug_lines( - debug_context, - tcx, - symbol, - context, - function_span, - source_info_set, - ); + let end = self.create_debug_lines(debug_context, symbol, context); debug_context.unit_range_list.0.push(Range::StartLength { begin: Address::Symbol { symbol, addend: 0 }, From 535c6ddc8bf66d8997c9a89ce457974e2c2c11ec Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Thu, 18 Aug 2022 18:59:14 +0000 Subject: [PATCH 15/15] Small cleanup --- src/driver/aot.rs | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/driver/aot.rs b/src/driver/aot.rs index 971e5eeea..3220d16f7 100644 --- a/src/driver/aot.rs +++ b/src/driver/aot.rs @@ -280,25 +280,20 @@ fn module_codegen( cgu.is_primary(), ); - let global_asm_object_file = match crate::global_asm::compile_global_asm( + let global_asm_object_file = crate::global_asm::compile_global_asm( &global_asm_config, cgu.name().as_str(), &cx.global_asm, - ) { - Ok(global_asm_object_file) => global_asm_object_file, - Err(err) => tcx.sess.fatal(&err), - }; + )?; - let debug_context = cx.debug_context; - let unwind_context = cx.unwind_context; tcx.sess.time("write object file", || { emit_cgu( &global_asm_config.output_filenames, - &tcx.sess.prof, + &cx.profiler, cgu.name().as_str().to_string(), module, - debug_context, - unwind_context, + cx.debug_context, + cx.unwind_context, global_asm_object_file, ) })