Move Span lowering from debuginfo finalization to codegen

This removes the dependency on TyCtxt from the debuginfo finalization
code.

Author: bjorn3

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<FunctionDebugContext>,
-    function_span: Span,
-    source_info_set: IndexSet<SourceInfo>,
 }
 
 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);

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<CValue<'tcx>>,
 
     pub(crate) clif_comments: crate::pretty_clif::CommentWriter,
-    pub(crate) source_info_set: indexmap::IndexSet<SourceInfo>,
+
+    /// Last accessed source file and it's debuginfo file id.
+    ///
+    /// For optimization purposes only
+    pub(crate) last_source_file: Option<(Lrc<SourceFile>, 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

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<SourceFile>, 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<FileInfo> {
@@ -85,6 +61,38 @@ fn make_file_info(hash: SourceFileHash) -> Option<FileInfo> {
 }
 
 impl DebugContext {
+    pub(crate) fn get_span_loc(
+        tcx: TyCtxt<'_>,
+        function_span: Span,
+        span: Span,
+    ) -> (Lrc<SourceFile>, 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<SourceInfo>,
     ) -> 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;
         }

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,34 +123,28 @@ 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(),
+        }
     }
 }
 
 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<SourceInfo>,
     ) {
         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 },