From 83ef18cd6cdcf80b7075424625123f878bda8722 Mon Sep 17 00:00:00 2001
From: Zalathar <Zalathar@users.noreply.github.com>
Date: Wed, 24 Jan 2024 12:57:11 +1100
Subject: [PATCH 1/2] coverage: Dismantle `Instrumentor` into ordinary
functions
---
.../rustc_mir_transform/src/coverage/mod.rs | 265 +++++++++---------
1 file changed, 126 insertions(+), 139 deletions(-)
diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs
index a11d224e8f1b5..d40b625be88a2 100644
--- a/compiler/rustc_mir_transform/src/coverage/mod.rs
+++ b/compiler/rustc_mir_transform/src/coverage/mod.rs
@@ -59,167 +59,154 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage {
_ => {}
}
- trace!("InstrumentCoverage starting for {def_id:?}");
- Instrumentor::new(tcx, mir_body).inject_counters();
- trace!("InstrumentCoverage done for {def_id:?}");
+ instrument_function_for_coverage(tcx, mir_body);
}
}
-struct Instrumentor<'a, 'tcx> {
- tcx: TyCtxt<'tcx>,
- mir_body: &'a mut mir::Body<'tcx>,
- hir_info: ExtractedHirInfo,
- basic_coverage_blocks: CoverageGraph,
-}
-
-impl<'a, 'tcx> Instrumentor<'a, 'tcx> {
- fn new(tcx: TyCtxt<'tcx>, mir_body: &'a mut mir::Body<'tcx>) -> Self {
- let hir_info = extract_hir_info(tcx, mir_body.source.def_id().expect_local());
+fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir::Body<'tcx>) {
+ let def_id = mir_body.source.def_id();
+ let _span = debug_span!("instrument_function_for_coverage", ?def_id).entered();
- debug!(?hir_info, "instrumenting {:?}", mir_body.source.def_id());
+ let hir_info = extract_hir_info(tcx, def_id.expect_local());
+ let basic_coverage_blocks = CoverageGraph::from_mir(mir_body);
- let basic_coverage_blocks = CoverageGraph::from_mir(mir_body);
+ ////////////////////////////////////////////////////
+ // Compute coverage spans from the `CoverageGraph`.
+ let Some(coverage_spans) =
+ CoverageSpans::generate_coverage_spans(mir_body, &hir_info, &basic_coverage_blocks)
+ else {
+ // No relevant spans were found in MIR, so skip instrumenting this function.
+ return;
+ };
- Self { tcx, mir_body, hir_info, basic_coverage_blocks }
+ ////////////////////////////////////////////////////
+ // Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure
+ // every coverage span has a `Counter` or `Expression` assigned to its `BasicCoverageBlock`
+ // and all `Expression` dependencies (operands) are also generated, for any other
+ // `BasicCoverageBlock`s not already associated with a coverage span.
+ let bcb_has_coverage_spans = |bcb| coverage_spans.bcb_has_coverage_spans(bcb);
+ let coverage_counters =
+ CoverageCounters::make_bcb_counters(&basic_coverage_blocks, bcb_has_coverage_spans);
+
+ let mappings = create_mappings(tcx, &hir_info, &coverage_spans, &coverage_counters);
+ if mappings.is_empty() {
+ // No spans could be converted into valid mappings, so skip this function.
+ debug!("no spans could be converted into valid mappings; skipping");
+ return;
}
- fn inject_counters(&'a mut self) {
- ////////////////////////////////////////////////////
- // Compute coverage spans from the `CoverageGraph`.
- let Some(coverage_spans) = CoverageSpans::generate_coverage_spans(
- self.mir_body,
- &self.hir_info,
- &self.basic_coverage_blocks,
- ) else {
- // No relevant spans were found in MIR, so skip instrumenting this function.
- return;
- };
-
- ////////////////////////////////////////////////////
- // Create an optimized mix of `Counter`s and `Expression`s for the `CoverageGraph`. Ensure
- // every coverage span has a `Counter` or `Expression` assigned to its `BasicCoverageBlock`
- // and all `Expression` dependencies (operands) are also generated, for any other
- // `BasicCoverageBlock`s not already associated with a coverage span.
- let bcb_has_coverage_spans = |bcb| coverage_spans.bcb_has_coverage_spans(bcb);
- let coverage_counters = CoverageCounters::make_bcb_counters(
- &self.basic_coverage_blocks,
- bcb_has_coverage_spans,
- );
+ inject_coverage_statements(
+ mir_body,
+ &basic_coverage_blocks,
+ bcb_has_coverage_spans,
+ &coverage_counters,
+ );
- let mappings = self.create_mappings(&coverage_spans, &coverage_counters);
- if mappings.is_empty() {
- // No spans could be converted into valid mappings, so skip this function.
- debug!("no spans could be converted into valid mappings; skipping");
- return;
- }
+ mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo {
+ function_source_hash: hir_info.function_source_hash,
+ num_counters: coverage_counters.num_counters(),
+ expressions: coverage_counters.into_expressions(),
+ mappings,
+ }));
+}
- self.inject_coverage_statements(bcb_has_coverage_spans, &coverage_counters);
+/// For each coverage span extracted from MIR, create a corresponding
+/// mapping.
+///
+/// Precondition: All BCBs corresponding to those spans have been given
+/// coverage counters.
+fn create_mappings<'tcx>(
+ tcx: TyCtxt<'tcx>,
+ hir_info: &ExtractedHirInfo,
+ coverage_spans: &CoverageSpans,
+ coverage_counters: &CoverageCounters,
+) -> Vec<Mapping> {
+ let source_map = tcx.sess.source_map();
+ let body_span = hir_info.body_span;
+
+ let source_file = source_map.lookup_source_file(body_span.lo());
+ use rustc_session::RemapFileNameExt;
+ let file_name = Symbol::intern(&source_file.name.for_codegen(tcx.sess).to_string_lossy());
+
+ let term_for_bcb = |bcb| {
+ coverage_counters
+ .bcb_counter(bcb)
+ .expect("all BCBs with spans were given counters")
+ .as_term()
+ };
- self.mir_body.function_coverage_info = Some(Box::new(FunctionCoverageInfo {
- function_source_hash: self.hir_info.function_source_hash,
- num_counters: coverage_counters.num_counters(),
- expressions: coverage_counters.into_expressions(),
- mappings,
- }));
- }
+ coverage_spans
+ .all_bcb_mappings()
+ .filter_map(|&BcbMapping { kind: bcb_mapping_kind, span }| {
+ let kind = match bcb_mapping_kind {
+ BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)),
+ };
+ let code_region = make_code_region(source_map, file_name, span, body_span)?;
+ Some(Mapping { kind, code_region })
+ })
+ .collect::<Vec<_>>()
+}
- /// For each coverage span extracted from MIR, create a corresponding
- /// mapping.
- ///
- /// Precondition: All BCBs corresponding to those spans have been given
- /// coverage counters.
- fn create_mappings(
- &self,
- coverage_spans: &CoverageSpans,
- coverage_counters: &CoverageCounters,
- ) -> Vec<Mapping> {
- let source_map = self.tcx.sess.source_map();
- let body_span = self.hir_info.body_span;
-
- let source_file = source_map.lookup_source_file(body_span.lo());
- use rustc_session::RemapFileNameExt;
- let file_name =
- Symbol::intern(&source_file.name.for_codegen(self.tcx.sess).to_string_lossy());
-
- let term_for_bcb = |bcb| {
- coverage_counters
- .bcb_counter(bcb)
- .expect("all BCBs with spans were given counters")
- .as_term()
+/// For each BCB node or BCB edge that has an associated coverage counter,
+/// inject any necessary coverage statements into MIR.
+fn inject_coverage_statements<'tcx>(
+ mir_body: &mut mir::Body<'tcx>,
+ basic_coverage_blocks: &CoverageGraph,
+ bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool,
+ coverage_counters: &CoverageCounters,
+) {
+ // Process the counters associated with BCB nodes.
+ for (bcb, counter_kind) in coverage_counters.bcb_node_counters() {
+ let do_inject = match counter_kind {
+ // Counter-increment statements always need to be injected.
+ BcbCounter::Counter { .. } => true,
+ // The only purpose of expression-used statements is to detect
+ // when a mapping is unreachable, so we only inject them for
+ // expressions with one or more mappings.
+ BcbCounter::Expression { .. } => bcb_has_coverage_spans(bcb),
};
-
- coverage_spans
- .all_bcb_mappings()
- .filter_map(|&BcbMapping { kind: bcb_mapping_kind, span }| {
- let kind = match bcb_mapping_kind {
- BcbMappingKind::Code(bcb) => MappingKind::Code(term_for_bcb(bcb)),
- };
- let code_region = make_code_region(source_map, file_name, span, body_span)?;
- Some(Mapping { kind, code_region })
- })
- .collect::<Vec<_>>()
+ if do_inject {
+ inject_statement(
+ mir_body,
+ make_mir_coverage_kind(counter_kind),
+ basic_coverage_blocks[bcb].leader_bb(),
+ );
+ }
}
- /// For each BCB node or BCB edge that has an associated coverage counter,
- /// inject any necessary coverage statements into MIR.
- fn inject_coverage_statements(
- &mut self,
- bcb_has_coverage_spans: impl Fn(BasicCoverageBlock) -> bool,
- coverage_counters: &CoverageCounters,
- ) {
- // Process the counters associated with BCB nodes.
- for (bcb, counter_kind) in coverage_counters.bcb_node_counters() {
- let do_inject = match counter_kind {
- // Counter-increment statements always need to be injected.
- BcbCounter::Counter { .. } => true,
- // The only purpose of expression-used statements is to detect
- // when a mapping is unreachable, so we only inject them for
- // expressions with one or more mappings.
- BcbCounter::Expression { .. } => bcb_has_coverage_spans(bcb),
- };
- if do_inject {
- inject_statement(
- self.mir_body,
- self.make_mir_coverage_kind(counter_kind),
- self.basic_coverage_blocks[bcb].leader_bb(),
- );
- }
+ // Process the counters associated with BCB edges.
+ for (from_bcb, to_bcb, counter_kind) in coverage_counters.bcb_edge_counters() {
+ let do_inject = match counter_kind {
+ // Counter-increment statements always need to be injected.
+ BcbCounter::Counter { .. } => true,
+ // BCB-edge expressions never have mappings, so they never need
+ // a corresponding statement.
+ BcbCounter::Expression { .. } => false,
+ };
+ if !do_inject {
+ continue;
}
- // Process the counters associated with BCB edges.
- for (from_bcb, to_bcb, counter_kind) in coverage_counters.bcb_edge_counters() {
- let do_inject = match counter_kind {
- // Counter-increment statements always need to be injected.
- BcbCounter::Counter { .. } => true,
- // BCB-edge expressions never have mappings, so they never need
- // a corresponding statement.
- BcbCounter::Expression { .. } => false,
- };
- if !do_inject {
- continue;
- }
+ // We need to inject a coverage statement into a new BB between the
+ // last BB of `from_bcb` and the first BB of `to_bcb`.
+ let from_bb = basic_coverage_blocks[from_bcb].last_bb();
+ let to_bb = basic_coverage_blocks[to_bcb].leader_bb();
- // We need to inject a coverage statement into a new BB between the
- // last BB of `from_bcb` and the first BB of `to_bcb`.
- let from_bb = self.basic_coverage_blocks[from_bcb].last_bb();
- let to_bb = self.basic_coverage_blocks[to_bcb].leader_bb();
-
- let new_bb = inject_edge_counter_basic_block(self.mir_body, from_bb, to_bb);
- debug!(
- "Edge {from_bcb:?} (last {from_bb:?}) -> {to_bcb:?} (leader {to_bb:?}) \
+ let new_bb = inject_edge_counter_basic_block(mir_body, from_bb, to_bb);
+ debug!(
+ "Edge {from_bcb:?} (last {from_bb:?}) -> {to_bcb:?} (leader {to_bb:?}) \
requires a new MIR BasicBlock {new_bb:?} for edge counter {counter_kind:?}",
- );
+ );
- // Inject a counter into the newly-created BB.
- inject_statement(self.mir_body, self.make_mir_coverage_kind(counter_kind), new_bb);
- }
+ // Inject a counter into the newly-created BB.
+ inject_statement(mir_body, make_mir_coverage_kind(counter_kind), new_bb);
}
+}
- fn make_mir_coverage_kind(&self, counter_kind: &BcbCounter) -> CoverageKind {
- match *counter_kind {
- BcbCounter::Counter { id } => CoverageKind::CounterIncrement { id },
- BcbCounter::Expression { id } => CoverageKind::ExpressionUsed { id },
- }
+fn make_mir_coverage_kind(counter_kind: &BcbCounter) -> CoverageKind {
+ match *counter_kind {
+ BcbCounter::Counter { id } => CoverageKind::CounterIncrement { id },
+ BcbCounter::Expression { id } => CoverageKind::ExpressionUsed { id },
}
}
From 572d7e9e69ca85c7a11e8730d9e4921d59691800 Mon Sep 17 00:00:00 2001
From: Zalathar <Zalathar@users.noreply.github.com>
Date: Wed, 24 Jan 2024 12:40:31 +1100
Subject: [PATCH 2/2] coverage: Flatten the functions for extracting/refining
coverage spans
Consolidating this code into flatter functions reduces the amount of
pointer-chasing required to read and modify it.
---
.../rustc_mir_transform/src/coverage/mod.rs | 2 +-
.../rustc_mir_transform/src/coverage/spans.rs | 119 +++++++-----------
.../src/coverage/spans/from_mir.rs | 6 +
3 files changed, 53 insertions(+), 74 deletions(-)
diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs
index d40b625be88a2..59a83e8d3568d 100644
--- a/compiler/rustc_mir_transform/src/coverage/mod.rs
+++ b/compiler/rustc_mir_transform/src/coverage/mod.rs
@@ -73,7 +73,7 @@ fn instrument_function_for_coverage<'tcx>(tcx: TyCtxt<'tcx>, mir_body: &mut mir:
////////////////////////////////////////////////////
// Compute coverage spans from the `CoverageGraph`.
let Some(coverage_spans) =
- CoverageSpans::generate_coverage_spans(mir_body, &hir_info, &basic_coverage_blocks)
+ spans::generate_coverage_spans(mir_body, &hir_info, &basic_coverage_blocks)
else {
// No relevant spans were found in MIR, so skip instrumenting this function.
return;
diff --git a/compiler/rustc_mir_transform/src/coverage/spans.rs b/compiler/rustc_mir_transform/src/coverage/spans.rs
index 81f6c8312061b..dee6a3b7143f5 100644
--- a/compiler/rustc_mir_transform/src/coverage/spans.rs
+++ b/compiler/rustc_mir_transform/src/coverage/spans.rs
@@ -26,45 +26,6 @@ pub(super) struct CoverageSpans {
}
impl CoverageSpans {
- /// Extracts coverage-relevant spans from MIR, and associates them with
- /// their corresponding BCBs.
- ///
- /// Returns `None` if no coverage-relevant spans could be extracted.
- pub(super) fn generate_coverage_spans(
- mir_body: &mir::Body<'_>,
- hir_info: &ExtractedHirInfo,
- basic_coverage_blocks: &CoverageGraph,
- ) -> Option<Self> {
- let mut mappings = vec![];
-
- let coverage_spans = CoverageSpansGenerator::generate_coverage_spans(
- mir_body,
- hir_info,
- basic_coverage_blocks,
- );
- mappings.extend(coverage_spans.into_iter().map(|CoverageSpan { bcb, span, .. }| {
- // Each span produced by the generator represents an ordinary code region.
- BcbMapping { kind: BcbMappingKind::Code(bcb), span }
- }));
-
- if mappings.is_empty() {
- return None;
- }
-
- // Identify which BCBs have one or more mappings.
- let mut bcb_has_mappings = BitSet::new_empty(basic_coverage_blocks.num_nodes());
- let mut insert = |bcb| {
- bcb_has_mappings.insert(bcb);
- };
- for &BcbMapping { kind, span: _ } in &mappings {
- match kind {
- BcbMappingKind::Code(bcb) => insert(bcb),
- }
- }
-
- Some(Self { bcb_has_mappings, mappings })
- }
-
pub(super) fn bcb_has_coverage_spans(&self, bcb: BasicCoverageBlock) -> bool {
self.bcb_has_mappings.contains(bcb)
}
@@ -74,6 +35,43 @@ impl CoverageSpans {
}
}
+/// Extracts coverage-relevant spans from MIR, and associates them with
+/// their corresponding BCBs.
+///
+/// Returns `None` if no coverage-relevant spans could be extracted.
+pub(super) fn generate_coverage_spans(
+ mir_body: &mir::Body<'_>,
+ hir_info: &ExtractedHirInfo,
+ basic_coverage_blocks: &CoverageGraph,
+) -> Option<CoverageSpans> {
+ let mut mappings = vec![];
+
+ let sorted_spans =
+ from_mir::mir_to_initial_sorted_coverage_spans(mir_body, hir_info, basic_coverage_blocks);
+ let coverage_spans = SpansRefiner::refine_sorted_spans(basic_coverage_blocks, sorted_spans);
+ mappings.extend(coverage_spans.into_iter().map(|CoverageSpan { bcb, span, .. }| {
+ // Each span produced by the generator represents an ordinary code region.
+ BcbMapping { kind: BcbMappingKind::Code(bcb), span }
+ }));
+
+ if mappings.is_empty() {
+ return None;
+ }
+
+ // Identify which BCBs have one or more mappings.
+ let mut bcb_has_mappings = BitSet::new_empty(basic_coverage_blocks.num_nodes());
+ let mut insert = |bcb| {
+ bcb_has_mappings.insert(bcb);
+ };
+ for &BcbMapping { kind, span: _ } in &mappings {
+ match kind {
+ BcbMappingKind::Code(bcb) => insert(bcb),
+ }
+ }
+
+ Some(CoverageSpans { bcb_has_mappings, mappings })
+}
+
/// A BCB is deconstructed into one or more `Span`s. Each `Span` maps to a `CoverageSpan` that
/// references the originating BCB and one or more MIR `Statement`s and/or `Terminator`s.
/// Initially, the `Span`s come from the `Statement`s and `Terminator`s, but subsequent
@@ -130,7 +128,7 @@ impl CoverageSpan {
/// * Merge spans that represent continuous (both in source code and control flow), non-branching
/// execution
/// * Carve out (leave uncovered) any span that will be counted by another MIR (notably, closures)
-struct CoverageSpansGenerator<'a> {
+struct SpansRefiner<'a> {
/// The BasicCoverageBlock Control Flow Graph (BCB CFG).
basic_coverage_blocks: &'a CoverageGraph,
@@ -173,40 +171,15 @@ struct CoverageSpansGenerator<'a> {
refined_spans: Vec<CoverageSpan>,
}
-impl<'a> CoverageSpansGenerator<'a> {
- /// Generate a minimal set of `CoverageSpan`s, each representing a contiguous code region to be
- /// counted.
- ///
- /// The basic steps are:
- ///
- /// 1. Extract an initial set of spans from the `Statement`s and `Terminator`s of each
- /// `BasicCoverageBlockData`.
- /// 2. Sort the spans by span.lo() (starting position). Spans that start at the same position
- /// are sorted with longer spans before shorter spans; and equal spans are sorted
- /// (deterministically) based on "dominator" relationship (if any).
- /// 3. Traverse the spans in sorted order to identify spans that can be dropped (for instance,
- /// if another span or spans are already counting the same code region), or should be merged
- /// into a broader combined span (because it represents a contiguous, non-branching, and
- /// uninterrupted region of source code).
- ///
- /// Closures are exposed in their enclosing functions as `Assign` `Rvalue`s, and since
- /// closures have their own MIR, their `Span` in their enclosing function should be left
- /// "uncovered".
- ///
- /// Note the resulting vector of `CoverageSpan`s may not be fully sorted (and does not need
- /// to be).
- pub(super) fn generate_coverage_spans(
- mir_body: &mir::Body<'_>,
- hir_info: &ExtractedHirInfo,
+impl<'a> SpansRefiner<'a> {
+ /// Takes the initial list of (sorted) spans extracted from MIR, and "refines"
+ /// them by merging compatible adjacent spans, removing redundant spans,
+ /// and carving holes in spans when they overlap in unwanted ways.
+ fn refine_sorted_spans(
basic_coverage_blocks: &'a CoverageGraph,
+ sorted_spans: Vec<CoverageSpan>,
) -> Vec<CoverageSpan> {
- let sorted_spans = from_mir::mir_to_initial_sorted_coverage_spans(
- mir_body,
- hir_info,
- basic_coverage_blocks,
- );
-
- let coverage_spans = Self {
+ let this = Self {
basic_coverage_blocks,
sorted_spans_iter: sorted_spans.into_iter(),
some_curr: None,
@@ -217,7 +190,7 @@ impl<'a> CoverageSpansGenerator<'a> {
refined_spans: Vec::with_capacity(basic_coverage_blocks.num_nodes() * 2),
};
- coverage_spans.to_refined_spans()
+ this.to_refined_spans()
}
/// Iterate through the sorted `CoverageSpan`s, and return the refined list of merged and
diff --git a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs
index 1b6dfccd57495..8d8e8e6132743 100644
--- a/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs
+++ b/compiler/rustc_mir_transform/src/coverage/spans/from_mir.rs
@@ -12,6 +12,12 @@ use crate::coverage::graph::{
use crate::coverage::spans::CoverageSpan;
use crate::coverage::ExtractedHirInfo;
+/// Traverses the MIR body to produce an initial collection of coverage-relevant
+/// spans, each associated with a node in the coverage graph (BCB) and possibly
+/// other metadata.
+///
+/// The returned spans are sorted in a specific order that is expected by the
+/// subsequent span-refinement step.
pub(super) fn mir_to_initial_sorted_coverage_spans(
mir_body: &mir::Body<'_>,
hir_info: &ExtractedHirInfo,