Skip to content

Commit

Permalink
Unrolled build for rust-lang#133418
Browse files Browse the repository at this point in the history 
Rollup merge of rust-lang#133418 - Zalathar:spans, r=jieyouxu

coverage: Store coverage source regions as `Span` until codegen

Historically, coverage spans were converted into line/column coordinates during the MIR instrumentation pass.

This PR moves that conversion step into codegen, so that coverage spans spend most of their time stored as `Span` instead.

In addition to being conceptually nicer, this also reduces the size of coverage mappings in MIR, because `Span` is smaller than 4x u32.

---

There should be no changes to coverage output.
  • Loading branch information
@rust-timer
rust-timer authored Nov 28, 2024
2 parents 66adeaf + 2748009 commit 1c8fdc4
Show file tree
Hide file tree
Showing 13 changed files with 251 additions and 285 deletions.
36 changes: 10 additions & 26 deletions compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId, SourceRegion};

use crate::coverageinfo::mapgen::LocalFileId;
use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId};

/// Must match the layout of `LLVMRustCounterKind`.
#[derive(Copy, Clone, Debug)]
Expand Down Expand Up @@ -126,45 +124,31 @@ pub(crate) struct CoverageSpan {
/// Local index into the function's local-to-global file ID table.
/// The value at that index is itself an index into the coverage filename
/// table in the CGU's `__llvm_covmap` section.
file_id: u32,
pub(crate) file_id: u32,

/// 1-based starting line of the source code span.
start_line: u32,
pub(crate) start_line: u32,
/// 1-based starting column of the source code span.
start_col: u32,
pub(crate) start_col: u32,
/// 1-based ending line of the source code span.
end_line: u32,
pub(crate) end_line: u32,
/// 1-based ending column of the source code span. High bit must be unset.
end_col: u32,
}

impl CoverageSpan {
pub(crate) fn from_source_region(
local_file_id: LocalFileId,
code_region: &SourceRegion,
) -> Self {
let file_id = local_file_id.as_u32();
let &SourceRegion { start_line, start_col, end_line, end_col } = code_region;
// Internally, LLVM uses the high bit of `end_col` to distinguish between
// code regions and gap regions, so it can't be used by the column number.
assert!(end_col & (1u32 << 31) == 0, "high bit of `end_col` must be unset: {end_col:#X}");
Self { file_id, start_line, start_col, end_line, end_col }
}
pub(crate) end_col: u32,
}

/// Must match the layout of `LLVMRustCoverageCodeRegion`.
#[derive(Clone, Debug)]
#[repr(C)]
pub(crate) struct CodeRegion {
pub(crate) span: CoverageSpan,
pub(crate) cov_span: CoverageSpan,
pub(crate) counter: Counter,
}

/// Must match the layout of `LLVMRustCoverageBranchRegion`.
#[derive(Clone, Debug)]
#[repr(C)]
pub(crate) struct BranchRegion {
pub(crate) span: CoverageSpan,
pub(crate) cov_span: CoverageSpan,
pub(crate) true_counter: Counter,
pub(crate) false_counter: Counter,
}
Expand All @@ -173,7 +157,7 @@ pub(crate) struct BranchRegion {
#[derive(Clone, Debug)]
#[repr(C)]
pub(crate) struct MCDCBranchRegion {
pub(crate) span: CoverageSpan,
pub(crate) cov_span: CoverageSpan,
pub(crate) true_counter: Counter,
pub(crate) false_counter: Counter,
pub(crate) mcdc_branch_params: mcdc::BranchParameters,
Expand All @@ -183,6 +167,6 @@ pub(crate) struct MCDCBranchRegion {
#[derive(Clone, Debug)]
#[repr(C)]
pub(crate) struct MCDCDecisionRegion {
pub(crate) span: CoverageSpan,
pub(crate) cov_span: CoverageSpan,
pub(crate) mcdc_decision_params: mcdc::DecisionParameters,
}
14 changes: 7 additions & 7 deletions compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ use rustc_data_structures::fx::FxIndexSet;
use rustc_index::bit_set::BitSet;
use rustc_middle::mir::coverage::{
CounterId, CovTerm, Expression, ExpressionId, FunctionCoverageInfo, Mapping, MappingKind, Op,
SourceRegion,
};
use rustc_middle::ty::Instance;
use rustc_span::Span;
use tracing::{debug, instrument};

use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind};
Expand Down Expand Up @@ -220,16 +220,16 @@ impl<'tcx> FunctionCoverage<'tcx> {
})
}

/// Converts this function's coverage mappings into an intermediate form
/// that will be used by `mapgen` when preparing for FFI.
pub(crate) fn counter_regions(
/// Yields all this function's coverage mappings, after simplifying away
/// unused counters and counter expressions.
pub(crate) fn mapping_spans(
&self,
) -> impl Iterator<Item = (MappingKind, &SourceRegion)> + ExactSizeIterator {
) -> impl Iterator<Item = (MappingKind, Span)> + ExactSizeIterator + Captures<'_> {
self.function_coverage_info.mappings.iter().map(move |mapping| {
let Mapping { kind, source_region } = mapping;
let &Mapping { ref kind, span } = mapping;
let kind =
kind.map_terms(|term| if self.is_zero_term(term) { CovTerm::Zero } else { term });
(kind, source_region)
(kind, span)
})
}

Expand Down
129 changes: 66 additions & 63 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
mod spans;

use std::ffi::CString;
use std::iter;
use std::sync::Arc;

use itertools::Itertools as _;
use rustc_abi::Align;
use rustc_codegen_ssa::traits::{
BaseTypeCodegenMethods, ConstCodegenMethods, StaticCodegenMethods,
};
use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_index::IndexVec;
use rustc_middle::mir::coverage::MappingKind;
Expand All @@ -15,7 +17,7 @@ use rustc_middle::{bug, mir};
use rustc_session::RemapFileNameExt;
use rustc_session::config::RemapPathScopeComponents;
use rustc_span::def_id::DefIdSet;
use rustc_span::{Span, Symbol};
use rustc_span::{SourceFile, StableSourceFileId};
use rustc_target::spec::HasTargetSpec;
use tracing::debug;

Expand Down Expand Up @@ -72,11 +74,11 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
.map(|(instance, function_coverage)| (instance, function_coverage.into_finished()))
.collect::<Vec<_>>();

let all_file_names = function_coverage_entries
let all_files = function_coverage_entries
.iter()
.map(|(_, fn_cov)| fn_cov.function_coverage_info.body_span)
.map(|span| span_file_name(tcx, span));
let global_file_table = GlobalFileTable::new(all_file_names);
.map(|span| tcx.sess.source_map().lookup_source_file(span.lo()));
let global_file_table = GlobalFileTable::new(all_files);

// Encode all filenames referenced by coverage mappings in this CGU.
let filenames_buffer = global_file_table.make_filenames_buffer(tcx);
Expand All @@ -103,15 +105,8 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
encode_mappings_for_function(tcx, &global_file_table, &function_coverage);

if coverage_mapping_buffer.is_empty() {
if function_coverage.is_used() {
bug!(
"A used function should have had coverage mapping data but did not: {}",
mangled_function_name
);
} else {
debug!("unused function had no coverage mapping data: {}", mangled_function_name);
continue;
}
debug!("function has no mappings to embed; skipping");
continue;
}

if !is_used {
Expand Down Expand Up @@ -148,54 +143,62 @@ pub(crate) fn finalize(cx: &CodegenCx<'_, '_>) {
}
}

/// Maps "global" (per-CGU) file ID numbers to their underlying filenames.
/// Maps "global" (per-CGU) file ID numbers to their underlying source files.
struct GlobalFileTable {
/// This "raw" table doesn't include the working dir, so a filename's
/// This "raw" table doesn't include the working dir, so a file's
/// global ID is its index in this set **plus one**.
raw_file_table: FxIndexSet<Symbol>,
raw_file_table: FxIndexMap<StableSourceFileId, Arc<SourceFile>>,
}

impl GlobalFileTable {
fn new(all_file_names: impl IntoIterator<Item = Symbol>) -> Self {
// Collect all of the filenames into a set. Filenames usually come in
// contiguous runs, so we can dedup adjacent ones to save work.
let mut raw_file_table = all_file_names.into_iter().dedup().collect::<FxIndexSet<Symbol>>();
fn new(all_files: impl IntoIterator<Item = Arc<SourceFile>>) -> Self {
// Collect all of the files into a set. Files usually come in contiguous
// runs, so we can dedup adjacent ones to save work.
let mut raw_file_table = all_files
.into_iter()
.dedup_by(|a, b| a.stable_id == b.stable_id)
.map(|f| (f.stable_id, f))
.collect::<FxIndexMap<StableSourceFileId, Arc<SourceFile>>>();

// Sort the file table by its actual string values, not the arbitrary
// ordering of its symbols.
raw_file_table.sort_unstable_by(|a, b| a.as_str().cmp(b.as_str()));
// Sort the file table by its underlying filenames.
raw_file_table.sort_unstable_by(|_, a, _, b| {
Ord::cmp(&a.name, &b.name).then_with(|| Ord::cmp(&a.stable_id, &b.stable_id))
});

Self { raw_file_table }
}

fn global_file_id_for_file_name(&self, file_name: Symbol) -> GlobalFileId {
let raw_id = self.raw_file_table.get_index_of(&file_name).unwrap_or_else(|| {
bug!("file name not found in prepared global file table: {file_name}");
fn global_file_id_for_file(&self, file: &SourceFile) -> GlobalFileId {
let raw_id = self.raw_file_table.get_index_of(&file.stable_id).unwrap_or_else(|| {
bug!("file not found in prepared global file table: {:?}", file.name);
});
// The raw file table doesn't include an entry for the working dir
// (which has ID 0), so add 1 to get the correct ID.
GlobalFileId::from_usize(raw_id + 1)
}

fn make_filenames_buffer(&self, tcx: TyCtxt<'_>) -> Vec<u8> {
let mut table = Vec::with_capacity(self.raw_file_table.len() + 1);

// LLVM Coverage Mapping Format version 6 (zero-based encoded as 5)
// requires setting the first filename to the compilation directory.
// Since rustc generates coverage maps with relative paths, the
// compilation directory can be combined with the relative paths
// to get absolute paths, if needed.
use rustc_session::RemapFileNameExt;
use rustc_session::config::RemapPathScopeComponents;
let working_dir: &str = &tcx
.sess
.opts
.working_dir
.for_scope(tcx.sess, RemapPathScopeComponents::MACRO)
.to_string_lossy();

// Insert the working dir at index 0, before the other filenames.
let filenames =
iter::once(working_dir).chain(self.raw_file_table.iter().map(Symbol::as_str));
llvm_cov::write_filenames_to_buffer(filenames)
table.push(
tcx.sess
.opts
.working_dir
.for_scope(tcx.sess, RemapPathScopeComponents::MACRO)
.to_string_lossy(),
);

// Add the regular entries after the base directory.
table.extend(self.raw_file_table.values().map(|file| {
file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy()
}));

llvm_cov::write_filenames_to_buffer(table.iter().map(|f| f.as_ref()))
}
}

Expand All @@ -208,7 +211,7 @@ rustc_index::newtype_index! {
/// An index into a function's list of global file IDs. That underlying list
/// of local-to-global mappings will be embedded in the function's record in
/// the `__llvm_covfun` linker section.
pub(crate) struct LocalFileId {}
struct LocalFileId {}
}

/// Holds a mapping from "local" (per-function) file IDs to "global" (per-CGU)
Expand All @@ -234,13 +237,6 @@ impl VirtualFileMapping {
}
}

fn span_file_name(tcx: TyCtxt<'_>, span: Span) -> Symbol {
let source_file = tcx.sess.source_map().lookup_source_file(span.lo());
let name =
source_file.name.for_scope(tcx.sess, RemapPathScopeComponents::MACRO).to_string_lossy();
Symbol::intern(&name)
}

/// Using the expressions and counter regions collected for a single function,
/// generate the variable-sized payload of its corresponding `__llvm_covfun`
/// entry. The payload is returned as a vector of bytes.
Expand All @@ -251,11 +247,13 @@ fn encode_mappings_for_function(
global_file_table: &GlobalFileTable,
function_coverage: &FunctionCoverage<'_>,
) -> Vec<u8> {
let counter_regions = function_coverage.counter_regions();
if counter_regions.is_empty() {
let mapping_spans = function_coverage.mapping_spans();
if mapping_spans.is_empty() {
return Vec::new();
}

let fn_cov_info = function_coverage.function_coverage_info;

let expressions = function_coverage.counter_expressions().collect::<Vec<_>>();

let mut virtual_file_mapping = VirtualFileMapping::default();
Expand All @@ -265,42 +263,47 @@ fn encode_mappings_for_function(
let mut mcdc_decision_regions = vec![];

// Currently a function's mappings must all be in the same file as its body span.
let file_name = span_file_name(tcx, function_coverage.function_coverage_info.body_span);
let source_map = tcx.sess.source_map();
let source_file = source_map.lookup_source_file(fn_cov_info.body_span.lo());

// Look up the global file ID for that filename.
let global_file_id = global_file_table.global_file_id_for_file_name(file_name);
// Look up the global file ID for that file.
let global_file_id = global_file_table.global_file_id_for_file(&source_file);

// Associate that global file ID with a local file ID for this function.
let local_file_id = virtual_file_mapping.local_id_for_global(global_file_id);
debug!(" file id: {local_file_id:?} => {global_file_id:?} = '{file_name:?}'");

// For each counter/region pair in this function+file, convert it to a
let make_cov_span = |span| {
spans::make_coverage_span(local_file_id, source_map, fn_cov_info, &source_file, span)
};

// For each coverage mapping span in this function+file, convert it to a
// form suitable for FFI.
for (mapping_kind, region) in counter_regions {
debug!("Adding counter {mapping_kind:?} to map for {region:?}");
let span = ffi::CoverageSpan::from_source_region(local_file_id, region);
for (mapping_kind, span) in mapping_spans {
debug!("Adding counter {mapping_kind:?} to map for {span:?}");
let Some(cov_span) = make_cov_span(span) else { continue };
match mapping_kind {
MappingKind::Code(term) => {
code_regions.push(ffi::CodeRegion { span, counter: ffi::Counter::from_term(term) });
code_regions
.push(ffi::CodeRegion { cov_span, counter: ffi::Counter::from_term(term) });
}
MappingKind::Branch { true_term, false_term } => {
branch_regions.push(ffi::BranchRegion {
span,
cov_span,
true_counter: ffi::Counter::from_term(true_term),
false_counter: ffi::Counter::from_term(false_term),
});
}
MappingKind::MCDCBranch { true_term, false_term, mcdc_params } => {
mcdc_branch_regions.push(ffi::MCDCBranchRegion {
span,
cov_span,
true_counter: ffi::Counter::from_term(true_term),
false_counter: ffi::Counter::from_term(false_term),
mcdc_branch_params: ffi::mcdc::BranchParameters::from(mcdc_params),
});
}
MappingKind::MCDCDecision(mcdc_decision_params) => {
mcdc_decision_regions.push(ffi::MCDCDecisionRegion {
span,
cov_span,
mcdc_decision_params: ffi::mcdc::DecisionParameters::from(mcdc_decision_params),
});
}
Expand Down
Loading

0 comments on commit 1c8fdc4

Please sign in to comment.