Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

coverage: Several small improvements to graph code #126538

Merged
merged 4 commits into from
Jun 17, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
coverage: Reduce/simplify visibility in coverage::graph
Using `pub(super)` makes it harder to move code between modules, and doesn't
provide much privacy benefit over `pub(crate)`.
  • Loading branch information
Zalathar committed Jun 16, 2024
commit 917b455a872050b90474043f31ef657b338526ba
52 changes: 28 additions & 24 deletions compiler/rustc_mir_transform/src/coverage/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@ use std::ops::{Index, IndexMut};
/// A coverage-specific simplification of the MIR control flow graph (CFG). The `CoverageGraph`s
/// nodes are `BasicCoverageBlock`s, which encompass one or more MIR `BasicBlock`s.
#[derive(Debug)]
pub(super) struct CoverageGraph {
pub(crate) struct CoverageGraph {
bcbs: IndexVec<BasicCoverageBlock, BasicCoverageBlockData>,
bb_to_bcb: IndexVec<BasicBlock, Option<BasicCoverageBlock>>,
pub successors: IndexVec<BasicCoverageBlock, Vec<BasicCoverageBlock>>,
pub predecessors: IndexVec<BasicCoverageBlock, Vec<BasicCoverageBlock>>,
pub(crate) successors: IndexVec<BasicCoverageBlock, Vec<BasicCoverageBlock>>,
pub(crate) predecessors: IndexVec<BasicCoverageBlock, Vec<BasicCoverageBlock>>,
dominators: Option<Dominators<BasicCoverageBlock>>,
}

impl CoverageGraph {
pub fn from_mir(mir_body: &mir::Body<'_>) -> Self {
pub(crate) fn from_mir(mir_body: &mir::Body<'_>) -> Self {
let (bcbs, bb_to_bcb) = Self::compute_basic_coverage_blocks(mir_body);

// Pre-transform MIR `BasicBlock` successors and predecessors into the BasicCoverageBlock
Expand Down Expand Up @@ -135,24 +135,28 @@ impl CoverageGraph {
}

#[inline(always)]
pub fn iter_enumerated(
pub(crate) fn iter_enumerated(
&self,
) -> impl Iterator<Item = (BasicCoverageBlock, &BasicCoverageBlockData)> {
self.bcbs.iter_enumerated()
}

#[inline(always)]
pub fn bcb_from_bb(&self, bb: BasicBlock) -> Option<BasicCoverageBlock> {
pub(crate) fn bcb_from_bb(&self, bb: BasicBlock) -> Option<BasicCoverageBlock> {
if bb.index() < self.bb_to_bcb.len() { self.bb_to_bcb[bb] } else { None }
}

#[inline(always)]
pub fn dominates(&self, dom: BasicCoverageBlock, node: BasicCoverageBlock) -> bool {
pub(crate) fn dominates(&self, dom: BasicCoverageBlock, node: BasicCoverageBlock) -> bool {
self.dominators.as_ref().unwrap().dominates(dom, node)
}

#[inline(always)]
pub fn cmp_in_dominator_order(&self, a: BasicCoverageBlock, b: BasicCoverageBlock) -> Ordering {
pub(crate) fn cmp_in_dominator_order(
&self,
a: BasicCoverageBlock,
b: BasicCoverageBlock,
) -> Ordering {
self.dominators.as_ref().unwrap().cmp_in_dominator_order(a, b)
}

Expand All @@ -166,7 +170,7 @@ impl CoverageGraph {
///
/// FIXME: That assumption might not be true for [`TerminatorKind::Yield`]?
#[inline(always)]
pub(super) fn bcb_has_multiple_in_edges(&self, bcb: BasicCoverageBlock) -> bool {
pub(crate) fn bcb_has_multiple_in_edges(&self, bcb: BasicCoverageBlock) -> bool {
// Even though bcb0 conceptually has an extra virtual in-edge due to
// being the entry point, we've already asserted that it has no _other_
// in-edges, so there's no possibility of it having _multiple_ in-edges.
Expand Down Expand Up @@ -227,7 +231,7 @@ rustc_index::newtype_index! {
/// A node in the control-flow graph of CoverageGraph.
#[orderable]
#[debug_format = "bcb{}"]
pub(super) struct BasicCoverageBlock {
pub(crate) struct BasicCoverageBlock {
const START_BCB = 0;
}
}
Expand Down Expand Up @@ -259,23 +263,23 @@ rustc_index::newtype_index! {
/// queries (`dominates()`, `predecessors`, `successors`, etc.) have branch (control flow)
/// significance.
#[derive(Debug, Clone)]
pub(super) struct BasicCoverageBlockData {
pub basic_blocks: Vec<BasicBlock>,
pub(crate) struct BasicCoverageBlockData {
pub(crate) basic_blocks: Vec<BasicBlock>,
}

impl BasicCoverageBlockData {
pub fn from(basic_blocks: Vec<BasicBlock>) -> Self {
fn from(basic_blocks: Vec<BasicBlock>) -> Self {
assert!(basic_blocks.len() > 0);
Self { basic_blocks }
}

#[inline(always)]
pub fn leader_bb(&self) -> BasicBlock {
pub(crate) fn leader_bb(&self) -> BasicBlock {
self.basic_blocks[0]
}

#[inline(always)]
pub fn last_bb(&self) -> BasicBlock {
pub(crate) fn last_bb(&self) -> BasicBlock {
*self.basic_blocks.last().unwrap()
}
}
Expand Down Expand Up @@ -364,7 +368,7 @@ fn bcb_filtered_successors<'a, 'tcx>(terminator: &'a Terminator<'tcx>) -> Covera
/// CoverageGraph outside all loops. This supports traversing the BCB CFG in a way that
/// ensures a loop is completely traversed before processing Blocks after the end of the loop.
#[derive(Debug)]
pub(super) struct TraversalContext {
struct TraversalContext {
/// BCB with one or more incoming loop backedges, indicating which loop
/// this context is for.
///
Expand All @@ -375,7 +379,7 @@ pub(super) struct TraversalContext {
worklist: VecDeque<BasicCoverageBlock>,
}

pub(super) struct TraverseCoverageGraphWithLoops<'a> {
pub(crate) struct TraverseCoverageGraphWithLoops<'a> {
basic_coverage_blocks: &'a CoverageGraph,

backedges: IndexVec<BasicCoverageBlock, Vec<BasicCoverageBlock>>,
Expand All @@ -384,7 +388,7 @@ pub(super) struct TraverseCoverageGraphWithLoops<'a> {
}

impl<'a> TraverseCoverageGraphWithLoops<'a> {
pub(super) fn new(basic_coverage_blocks: &'a CoverageGraph) -> Self {
pub(crate) fn new(basic_coverage_blocks: &'a CoverageGraph) -> Self {
let backedges = find_loop_backedges(basic_coverage_blocks);

let worklist = VecDeque::from([basic_coverage_blocks.start_node()]);
Expand All @@ -400,15 +404,15 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> {

/// For each loop on the loop context stack (top-down), yields a list of BCBs
/// within that loop that have an outgoing edge back to the loop header.
pub(super) fn reloop_bcbs_per_loop(&self) -> impl Iterator<Item = &[BasicCoverageBlock]> {
pub(crate) fn reloop_bcbs_per_loop(&self) -> impl Iterator<Item = &[BasicCoverageBlock]> {
self.context_stack
.iter()
.rev()
.filter_map(|context| context.loop_header)
.map(|header_bcb| self.backedges[header_bcb].as_slice())
}

pub(super) fn next(&mut self) -> Option<BasicCoverageBlock> {
pub(crate) fn next(&mut self) -> Option<BasicCoverageBlock> {
debug!(
"TraverseCoverageGraphWithLoops::next - context_stack: {:?}",
self.context_stack.iter().rev().collect::<Vec<_>>()
Expand Down Expand Up @@ -440,7 +444,7 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> {
None
}

pub fn add_successors_to_worklists(&mut self, bcb: BasicCoverageBlock) {
fn add_successors_to_worklists(&mut self, bcb: BasicCoverageBlock) {
let successors = &self.basic_coverage_blocks.successors[bcb];
debug!("{:?} has {} successors:", bcb, successors.len());

Expand Down Expand Up @@ -494,19 +498,19 @@ impl<'a> TraverseCoverageGraphWithLoops<'a> {
}
}

pub fn is_complete(&self) -> bool {
pub(crate) fn is_complete(&self) -> bool {
self.visited.count() == self.visited.domain_size()
}

pub fn unvisited(&self) -> Vec<BasicCoverageBlock> {
pub(crate) fn unvisited(&self) -> Vec<BasicCoverageBlock> {
let mut unvisited_set: BitSet<BasicCoverageBlock> =
BitSet::new_filled(self.visited.domain_size());
unvisited_set.subtract(&self.visited);
unvisited_set.iter().collect::<Vec<_>>()
}
}

pub(super) fn find_loop_backedges(
fn find_loop_backedges(
basic_coverage_blocks: &CoverageGraph,
) -> IndexVec<BasicCoverageBlock, Vec<BasicCoverageBlock>> {
let num_bcbs = basic_coverage_blocks.num_nodes();
Expand Down