From 79f935b96c2447c979124628187125a9e381e9dc Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 31 Aug 2023 16:03:12 +1000 Subject: [PATCH] coverage: Rename `Operand` to `CovTerm` Later patches in this PR will use `CovTerm` to represent things that are not expression operands. --- .../src/coverageinfo/ffi.rs | 12 +++--- .../src/coverageinfo/map_data.rs | 32 +++++++-------- compiler/rustc_middle/src/mir/coverage.rs | 16 ++++---- .../src/coverage/counters.rs | 40 +++++++++---------- 4 files changed, 51 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs index 763186a58bf..194ed7d661d 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/ffi.rs @@ -1,4 +1,4 @@ -use rustc_middle::mir::coverage::{CounterId, ExpressionId, Operand}; +use rustc_middle::mir::coverage::{CounterId, CovTerm, ExpressionId}; /// Must match the layout of `LLVMRustCounterKind`. #[derive(Copy, Clone, Debug)] @@ -43,11 +43,11 @@ impl Counter { Self { kind: CounterKind::Expression, id: expression_id.as_u32() } } - pub(crate) fn from_operand(operand: Operand) -> Self { - match operand { - Operand::Zero => Self::ZERO, - Operand::Counter(id) => Self::counter_value_reference(id), - Operand::Expression(id) => Self::expression(id), + pub(crate) fn from_term(term: CovTerm) -> Self { + match term { + CovTerm::Zero => Self::ZERO, + CovTerm::Counter(id) => Self::counter_value_reference(id), + CovTerm::Expression(id) => Self::expression(id), } } } diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs index 4dbc0e403fd..9798c18f21b 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs @@ -3,15 +3,15 @@ use crate::coverageinfo::ffi::{Counter, CounterExpression, ExprKind}; use rustc_data_structures::fx::FxIndexSet; use rustc_index::IndexVec; use rustc_middle::mir::coverage::{ - CodeRegion, CounterId, ExpressionId, FunctionCoverageInfo, Op, Operand, + CodeRegion, CounterId, CovTerm, ExpressionId, FunctionCoverageInfo, Op, }; use rustc_middle::ty::Instance; #[derive(Clone, Debug, PartialEq)] pub struct Expression { - lhs: Operand, + lhs: CovTerm, op: Op, - rhs: Operand, + rhs: CovTerm, code_regions: Vec, } @@ -101,15 +101,15 @@ impl<'tcx> FunctionCoverage<'tcx> { /// code regions mapped to that expression. /// /// Both counters and "counter expressions" (or simply, "expressions") can be operands in other - /// expressions. These are tracked as separate variants of `Operand`, so there is no ambiguity + /// expressions. These are tracked as separate variants of `CovTerm`, so there is no ambiguity /// between operands that are counter IDs and operands that are expression IDs. #[instrument(level = "debug", skip(self))] pub(crate) fn add_counter_expression( &mut self, expression_id: ExpressionId, - lhs: Operand, + lhs: CovTerm, op: Op, - rhs: Operand, + rhs: CovTerm, code_regions: &[CodeRegion], ) { debug_assert!( @@ -151,7 +151,7 @@ impl<'tcx> FunctionCoverage<'tcx> { // The set of expressions that either were optimized out entirely, or // have zero as both of their operands, and will therefore always have // a value of zero. Other expressions that refer to these as operands - // can have those operands replaced with `Operand::Zero`. + // can have those operands replaced with `CovTerm::Zero`. let mut zero_expressions = FxIndexSet::default(); // For each expression, perform simplifications based on lower-numbered @@ -168,10 +168,10 @@ impl<'tcx> FunctionCoverage<'tcx> { }; // If an operand refers to an expression that is always zero, then - // that operand can be replaced with `Operand::Zero`. - let maybe_set_operand_to_zero = |operand: &mut Operand| match &*operand { - Operand::Expression(id) if zero_expressions.contains(id) => { - *operand = Operand::Zero; + // that operand can be replaced with `CovTerm::Zero`. + let maybe_set_operand_to_zero = |operand: &mut CovTerm| match &*operand { + CovTerm::Expression(id) if zero_expressions.contains(id) => { + *operand = CovTerm::Zero; } _ => (), }; @@ -181,13 +181,13 @@ impl<'tcx> FunctionCoverage<'tcx> { // Coverage counter values cannot be negative, so if an expression // involves subtraction from zero, assume that its RHS must also be zero. // (Do this after simplifications that could set the LHS to zero.) - if let Expression { lhs: Operand::Zero, op: Op::Subtract, .. } = expression { - expression.rhs = Operand::Zero; + if let Expression { lhs: CovTerm::Zero, op: Op::Subtract, .. } = expression { + expression.rhs = CovTerm::Zero; } // After the above simplifications, if both operands are zero, then // we know that this expression is always zero too. - if let Expression { lhs: Operand::Zero, rhs: Operand::Zero, .. } = expression { + if let Expression { lhs: CovTerm::Zero, rhs: CovTerm::Zero, .. } = expression { zero_expressions.insert(id); } } @@ -254,12 +254,12 @@ impl<'tcx> FunctionCoverage<'tcx> { &Some(Expression { lhs, op, rhs, .. }) => { // Convert the operands and operator as normal. CounterExpression::new( - Counter::from_operand(lhs), + Counter::from_term(lhs), match op { Op::Add => ExprKind::Add, Op::Subtract => ExprKind::Subtract, }, - Counter::from_operand(rhs), + Counter::from_term(rhs), ) } }) diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 837331a2de8..fbb7f33a983 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -35,19 +35,21 @@ impl ExpressionId { pub const START: Self = Self::from_u32(0); } -/// Operand of a coverage-counter expression. +/// Enum that can hold a constant zero value, the ID of an physical coverage +/// counter, or the ID of a coverage-counter expression. /// -/// Operands can be a constant zero value, an actual coverage counter, or another -/// expression. Counter/expression operands are referred to by ID. +/// This was originally only used for expression operands (and named `Operand`), +/// but the zero/counter/expression distinction is also useful for representing +/// the value of code/gap mappings, and the true/false arms of branch mappings. #[derive(Copy, Clone, PartialEq, Eq)] #[derive(TyEncodable, TyDecodable, Hash, HashStable, TypeFoldable, TypeVisitable)] -pub enum Operand { +pub enum CovTerm { Zero, Counter(CounterId), Expression(ExpressionId), } -impl Debug for Operand { +impl Debug for CovTerm { fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result { match self { Self::Zero => write!(f, "Zero"), @@ -68,9 +70,9 @@ pub enum CoverageKind { /// ID of this coverage-counter expression within its enclosing function. /// Other expressions in the same function can refer to it as an operand. id: ExpressionId, - lhs: Operand, + lhs: CovTerm, op: Op, - rhs: Operand, + rhs: CovTerm, }, Unreachable, } diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 938c4b2bd3f..4140d402c75 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -19,7 +19,7 @@ const NESTED_INDENT: &str = " "; #[derive(Clone)] pub(super) enum BcbCounter { Counter { id: CounterId }, - Expression { id: ExpressionId, lhs: Operand, op: Op, rhs: Operand }, + Expression { id: ExpressionId, lhs: CovTerm, op: Op, rhs: CovTerm }, } impl BcbCounter { @@ -27,10 +27,10 @@ impl BcbCounter { matches!(self, Self::Expression { .. }) } - pub(super) fn as_operand(&self) -> Operand { + pub(super) fn as_term(&self) -> CovTerm { match *self { - BcbCounter::Counter { id, .. } => Operand::Counter(id), - BcbCounter::Expression { id, .. } => Operand::Expression(id), + BcbCounter::Counter { id, .. } => CovTerm::Counter(id), + BcbCounter::Expression { id, .. } => CovTerm::Expression(id), } } } @@ -106,7 +106,7 @@ impl CoverageCounters { BcbCounter::Counter { id } } - fn make_expression(&mut self, lhs: Operand, op: Op, rhs: Operand) -> BcbCounter { + fn make_expression(&mut self, lhs: CovTerm, op: Op, rhs: CovTerm) -> BcbCounter { let id = self.next_expression(); BcbCounter::Expression { id, lhs, op, rhs } } @@ -138,7 +138,7 @@ impl CoverageCounters { &mut self, bcb: BasicCoverageBlock, counter_kind: BcbCounter, - ) -> Result { + ) -> Result { debug_assert!( // If the BCB has an edge counter (to be injected into a new `BasicBlock`), it can also // have an expression (to be injected into an existing `BasicBlock` represented by this @@ -146,14 +146,14 @@ impl CoverageCounters { counter_kind.is_expression() || !self.bcb_has_incoming_edge_counters.contains(bcb), "attempt to add a `Counter` to a BCB target with existing incoming edge counters" ); - let operand = counter_kind.as_operand(); + let term = counter_kind.as_term(); if let Some(replaced) = self.bcb_counters[bcb].replace(counter_kind) { Error::from_string(format!( "attempt to set a BasicCoverageBlock coverage counter more than once; \ {bcb:?} already had counter {replaced:?}", )) } else { - Ok(operand) + Ok(term) } } @@ -162,7 +162,7 @@ impl CoverageCounters { from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, counter_kind: BcbCounter, - ) -> Result { + ) -> Result { if level_enabled!(tracing::Level::DEBUG) { // If the BCB has an edge counter (to be injected into a new `BasicBlock`), it can also // have an expression (to be injected into an existing `BasicBlock` represented by this @@ -175,14 +175,14 @@ impl CoverageCounters { } } self.bcb_has_incoming_edge_counters.insert(to_bcb); - let operand = counter_kind.as_operand(); + let term = counter_kind.as_term(); if let Some(replaced) = self.bcb_edge_counters.insert((from_bcb, to_bcb), counter_kind) { Error::from_string(format!( "attempt to set an edge counter more than once; from_bcb: \ {from_bcb:?} already had counter {replaced:?}", )) } else { - Ok(operand) + Ok(term) } } @@ -284,7 +284,7 @@ impl<'a> MakeBcbCounters<'a> { &mut self, traversal: &TraverseCoverageGraphWithLoops<'_>, branching_bcb: BasicCoverageBlock, - branching_counter_operand: Operand, + branching_counter_operand: CovTerm, ) -> Result<(), Error> { let branches = self.bcb_branches(branching_bcb); debug!( @@ -332,7 +332,7 @@ impl<'a> MakeBcbCounters<'a> { sumup_counter_operand, ); debug!(" [new intermediate expression: {:?}]", intermediate_expression); - let intermediate_expression_operand = intermediate_expression.as_operand(); + let intermediate_expression_operand = intermediate_expression.as_term(); self.coverage_counters.intermediate_expressions.push(intermediate_expression); some_sumup_counter_operand.replace(intermediate_expression_operand); } @@ -364,7 +364,7 @@ impl<'a> MakeBcbCounters<'a> { Ok(()) } - fn get_or_make_counter_operand(&mut self, bcb: BasicCoverageBlock) -> Result { + fn get_or_make_counter_operand(&mut self, bcb: BasicCoverageBlock) -> Result { self.recursive_get_or_make_counter_operand(bcb, 1) } @@ -372,7 +372,7 @@ impl<'a> MakeBcbCounters<'a> { &mut self, bcb: BasicCoverageBlock, debug_indent_level: usize, - ) -> Result { + ) -> Result { // If the BCB already has a counter, return it. if let Some(counter_kind) = &self.coverage_counters.bcb_counters[bcb] { debug!( @@ -381,7 +381,7 @@ impl<'a> MakeBcbCounters<'a> { bcb, counter_kind, ); - return Ok(counter_kind.as_operand()); + return Ok(counter_kind.as_term()); } // A BCB with only one incoming edge gets a simple `Counter` (via `make_counter()`). @@ -445,7 +445,7 @@ impl<'a> MakeBcbCounters<'a> { NESTED_INDENT.repeat(debug_indent_level), intermediate_expression ); - let intermediate_expression_operand = intermediate_expression.as_operand(); + let intermediate_expression_operand = intermediate_expression.as_term(); self.coverage_counters.intermediate_expressions.push(intermediate_expression); some_sumup_edge_counter_operand.replace(intermediate_expression_operand); } @@ -468,7 +468,7 @@ impl<'a> MakeBcbCounters<'a> { &mut self, from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, - ) -> Result { + ) -> Result { self.recursive_get_or_make_edge_counter_operand(from_bcb, to_bcb, 1) } @@ -477,7 +477,7 @@ impl<'a> MakeBcbCounters<'a> { from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, debug_indent_level: usize, - ) -> Result { + ) -> Result { // If the source BCB has only one successor (assumed to be the given target), an edge // counter is unnecessary. Just get or make a counter for the source BCB. let successors = self.bcb_successors(from_bcb).iter(); @@ -496,7 +496,7 @@ impl<'a> MakeBcbCounters<'a> { to_bcb, counter_kind ); - return Ok(counter_kind.as_operand()); + return Ok(counter_kind.as_term()); } // Make a new counter to count this edge.