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

Consistently use dominates instead of is_dominated_by #107153

Merged
merged 1 commit into from
Jan 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion compiler/rustc_data_structures/src/graph/dominators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ impl<Node: Idx> Dominators<Node> {
Iter { dominators: self, node: Some(node) }
}

pub fn is_dominated_by(&self, node: Node, dom: Node) -> bool {
pub fn dominates(&self, dom: Node, node: Node) -> bool {
// FIXME -- could be optimized by using post-order-rank
self.dominators(node).any(|n| n == dom)
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3049,7 +3049,7 @@ impl Location {
if self.block == other.block {
self.statement_index <= other.statement_index
} else {
dominators.is_dominated_by(other.block, self.block)
dominators.dominates(self.block, other.block)
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_mir_transform/src/coverage/counters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,7 +520,7 @@ impl<'a> BcbCounters<'a> {
let mut found_loop_exit = false;
for &branch in branches.iter() {
if backedge_from_bcbs.iter().any(|&backedge_from_bcb| {
self.bcb_is_dominated_by(backedge_from_bcb, branch.target_bcb)
self.bcb_dominates(branch.target_bcb, backedge_from_bcb)
}) {
if let Some(reloop_branch) = some_reloop_branch {
if reloop_branch.counter(&self.basic_coverage_blocks).is_none() {
Expand Down Expand Up @@ -603,8 +603,8 @@ impl<'a> BcbCounters<'a> {
}

#[inline]
fn bcb_is_dominated_by(&self, node: BasicCoverageBlock, dom: BasicCoverageBlock) -> bool {
self.basic_coverage_blocks.is_dominated_by(node, dom)
fn bcb_dominates(&self, dom: BasicCoverageBlock, node: BasicCoverageBlock) -> bool {
self.basic_coverage_blocks.dominates(dom, node)
}

#[inline]
Expand Down
14 changes: 7 additions & 7 deletions compiler/rustc_mir_transform/src/coverage/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ impl CoverageGraph {
}

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

#[inline(always)]
Expand Down Expand Up @@ -312,7 +312,7 @@ rustc_index::newtype_index! {
/// to the BCB's primary counter or expression).
///
/// The BCB CFG is critical to simplifying the coverage analysis by ensuring graph path-based
/// queries (`is_dominated_by()`, `predecessors`, `successors`, etc.) have branch (control flow)
/// queries (`dominates()`, `predecessors`, `successors`, etc.) have branch (control flow)
/// significance.
#[derive(Debug, Clone)]
pub(super) struct BasicCoverageBlockData {
Expand Down Expand Up @@ -594,7 +594,7 @@ impl TraverseCoverageGraphWithLoops {
// branching block would have given an `Expression` (or vice versa).
let (some_successor_to_add, some_loop_header) =
if let Some((_, loop_header)) = context.loop_backedges {
if basic_coverage_blocks.is_dominated_by(successor, loop_header) {
if basic_coverage_blocks.dominates(loop_header, successor) {
(Some(successor), Some(loop_header))
} else {
(None, None)
Expand Down Expand Up @@ -666,15 +666,15 @@ pub(super) fn find_loop_backedges(
//
// The overall complexity appears to be comparable to many other MIR transform algorithms, and I
// don't expect that this function is creating a performance hot spot, but if this becomes an
// issue, there may be ways to optimize the `is_dominated_by` algorithm (as indicated by an
// issue, there may be ways to optimize the `dominates` algorithm (as indicated by an
// existing `FIXME` comment in that code), or possibly ways to optimize it's usage here, perhaps
// by keeping track of results for visited `BasicCoverageBlock`s if they can be used to short
// circuit downstream `is_dominated_by` checks.
// circuit downstream `dominates` checks.
//
// For now, that kind of optimization seems unnecessarily complicated.
for (bcb, _) in basic_coverage_blocks.iter_enumerated() {
for &successor in &basic_coverage_blocks.successors[bcb] {
if basic_coverage_blocks.is_dominated_by(bcb, successor) {
if basic_coverage_blocks.dominates(successor, bcb) {
let loop_header = successor;
let backedge_from_bcb = bcb;
debug!(
Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_mir_transform/src/coverage/spans.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl CoverageStatement {
/// Note: A `CoverageStatement` merged into another CoverageSpan may come from a `BasicBlock` that
/// is not part of the `CoverageSpan` bcb if the statement was included because it's `Span` matches
/// or is subsumed by the `Span` associated with this `CoverageSpan`, and it's `BasicBlock`
/// `is_dominated_by()` the `BasicBlock`s in this `CoverageSpan`.
/// `dominates()` the `BasicBlock`s in this `CoverageSpan`.
#[derive(Debug, Clone)]
pub(super) struct CoverageSpan {
pub span: Span,
Expand Down Expand Up @@ -705,12 +705,12 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
fn hold_pending_dups_unless_dominated(&mut self) {
// Equal coverage spans are ordered by dominators before dominated (if any), so it should be
// impossible for `curr` to dominate any previous `CoverageSpan`.
debug_assert!(!self.span_bcb_is_dominated_by(self.prev(), self.curr()));
debug_assert!(!self.span_bcb_dominates(self.curr(), self.prev()));

let initial_pending_count = self.pending_dups.len();
if initial_pending_count > 0 {
let mut pending_dups = self.pending_dups.split_off(0);
pending_dups.retain(|dup| !self.span_bcb_is_dominated_by(self.curr(), dup));
pending_dups.retain(|dup| !self.span_bcb_dominates(dup, self.curr()));
self.pending_dups.append(&mut pending_dups);
if self.pending_dups.len() < initial_pending_count {
debug!(
Expand All @@ -721,7 +721,7 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
}
}

if self.span_bcb_is_dominated_by(self.curr(), self.prev()) {
if self.span_bcb_dominates(self.prev(), self.curr()) {
debug!(
" different bcbs but SAME spans, and prev dominates curr. Discard prev={:?}",
self.prev()
Expand Down Expand Up @@ -787,8 +787,8 @@ impl<'a, 'tcx> CoverageSpans<'a, 'tcx> {
}
}

fn span_bcb_is_dominated_by(&self, covspan: &CoverageSpan, dom_covspan: &CoverageSpan) -> bool {
self.basic_coverage_blocks.is_dominated_by(covspan.bcb, dom_covspan.bcb)
fn span_bcb_dominates(&self, dom_covspan: &CoverageSpan, covspan: &CoverageSpan) -> bool {
self.basic_coverage_blocks.dominates(dom_covspan.bcb, covspan.bcb)
}
}

Expand Down