diff options
Diffstat (limited to 'compiler')
5 files changed, 17 insertions, 183 deletions
diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs index 1290c6efb09..c53ea6d4666 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mapgen/covfun.rs @@ -54,7 +54,7 @@ pub(crate) fn prepare_covfun_record<'tcx>( let fn_cov_info = tcx.instance_mir(instance.def).function_coverage_info.as_deref()?; let ids_info = tcx.coverage_ids_info(instance.def)?; - let expressions = prepare_expressions(ids_info, is_used); + let expressions = prepare_expressions(ids_info); let mut covfun = CovfunRecord { mangled_function_name: tcx.symbol_name(instance).name, @@ -76,16 +76,8 @@ pub(crate) fn prepare_covfun_record<'tcx>( } /// Convert the function's coverage-counter expressions into a form suitable for FFI. -fn prepare_expressions(ids_info: &CoverageIdsInfo, is_used: bool) -> Vec<ffi::CounterExpression> { - // If any counters or expressions were removed by MIR opts, replace their - // terms with zero. - let counter_for_term = |term| { - if !is_used || ids_info.is_zero_term(term) { - ffi::Counter::ZERO - } else { - ffi::Counter::from_term(term) - } - }; +fn prepare_expressions(ids_info: &CoverageIdsInfo) -> Vec<ffi::CounterExpression> { + let counter_for_term = ffi::Counter::from_term; // We know that LLVM will optimize out any unused expressions before // producing the final coverage map, so there's no need to do the same @@ -133,13 +125,14 @@ fn fill_region_tables<'tcx>( // For each counter/region pair in this function+file, convert it to a // form suitable for FFI. - let is_zero_term = |term| !covfun.is_used || ids_info.is_zero_term(term); for &Mapping { ref kind, span } in &fn_cov_info.mappings { - // If the mapping refers to counters/expressions that were removed by - // MIR opts, replace those occurrences with zero. + // If this function is unused, replace all counters with zero. let counter_for_bcb = |bcb: BasicCoverageBlock| -> ffi::Counter { - let term = ids_info.term_for_bcb[bcb].expect("every BCB in a mapping was given a term"); - let term = if is_zero_term(term) { CovTerm::Zero } else { term }; + let term = if covfun.is_used { + ids_info.term_for_bcb[bcb].expect("every BCB in a mapping was given a term") + } else { + CovTerm::Zero + }; ffi::Counter::from_term(term) }; diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index e404db5e196..ea7f581a3cb 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -163,11 +163,9 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { CoverageKind::VirtualCounter { bcb } if let Some(&id) = ids_info.phys_counter_for_node.get(&bcb) => { - let num_counters = ids_info.num_counters_after_mir_opts(); - let fn_name = bx.get_pgo_func_name_var(instance); let hash = bx.const_u64(function_coverage_info.function_source_hash); - let num_counters = bx.const_u32(num_counters); + let num_counters = bx.const_u32(ids_info.num_counters); let index = bx.const_u32(id.as_u32()); debug!( "codegen intrinsic instrprof.increment(fn_name={:?}, hash={:?}, num_counters={:?}, index={:?})", diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 31b9eeb715b..8c6b11a681e 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -3,7 +3,6 @@ use std::fmt::{self, Debug, Formatter}; use rustc_data_structures::fx::FxIndexMap; -use rustc_index::bit_set::DenseBitSet; use rustc_index::{Idx, IndexVec}; use rustc_macros::{HashStable, TyDecodable, TyEncodable}; use rustc_span::Span; @@ -277,41 +276,12 @@ pub struct MCDCDecisionSpan { /// Returned by the `coverage_ids_info` query. #[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable)] pub struct CoverageIdsInfo { - pub counters_seen: DenseBitSet<CounterId>, - pub zero_expressions: DenseBitSet<ExpressionId>, - + pub num_counters: u32, pub phys_counter_for_node: FxIndexMap<BasicCoverageBlock, CounterId>, pub term_for_bcb: IndexVec<BasicCoverageBlock, Option<CovTerm>>, pub expressions: IndexVec<ExpressionId, Expression>, } -impl CoverageIdsInfo { - /// Coverage codegen needs to know how many coverage counters are ever - /// incremented within a function, so that it can set the `num-counters` - /// argument of the `llvm.instrprof.increment` intrinsic. - /// - /// This may be less than the highest counter ID emitted by the - /// InstrumentCoverage MIR pass, if the highest-numbered counter increments - /// were removed by MIR optimizations. - pub fn num_counters_after_mir_opts(&self) -> u32 { - // FIXME(Zalathar): Currently this treats an unused counter as "used" - // if its ID is less than that of the highest counter that really is - // used. Fixing this would require adding a renumbering step somewhere. - self.counters_seen.last_set_in(..).map_or(0, |max| max.as_u32() + 1) - } - - /// Returns `true` if the given term is known to have a value of zero, taking - /// into account knowledge of which counters are unused and which expressions - /// are always zero. - pub fn is_zero_term(&self, term: CovTerm) -> bool { - match term { - CovTerm::Zero => true, - CovTerm::Counter(id) => !self.counters_seen.contains(id), - CovTerm::Expression(id) => self.zero_expressions.contains(id), - } - } -} - rustc_index::newtype_index! { /// During the `InstrumentCoverage` MIR pass, a BCB is a node in the /// "coverage graph", which is a refinement of the MIR control-flow graph diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index ac0039fd007..6f9984d5d0a 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -135,7 +135,7 @@ pub(super) struct CoverageCounters { /// List of places where a counter-increment statement should be injected /// into MIR, each with its corresponding counter ID. pub(crate) phys_counter_for_node: FxIndexMap<BasicCoverageBlock, CounterId>, - next_counter_id: CounterId, + pub(crate) next_counter_id: CounterId, /// Coverage counters/expressions that are associated with individual BCBs. pub(crate) node_counters: IndexVec<BasicCoverageBlock, Option<CovTerm>>, diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index 995fa91e0a0..cd89fbe772d 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -1,11 +1,7 @@ use rustc_data_structures::captures::Captures; -use rustc_index::IndexSlice; use rustc_index::bit_set::DenseBitSet; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; -use rustc_middle::mir::coverage::{ - BasicCoverageBlock, CounterId, CovTerm, CoverageIdsInfo, CoverageKind, Expression, - ExpressionId, MappingKind, Op, -}; +use rustc_middle::mir::coverage::{BasicCoverageBlock, CoverageIdsInfo, CoverageKind, MappingKind}; use rustc_middle::mir::{Body, Statement, StatementKind}; use rustc_middle::ty::{self, TyCtxt}; use rustc_middle::util::Providers; @@ -134,44 +130,12 @@ fn coverage_ids_info<'tcx>( let node_counters = make_node_counters(&fn_cov_info.node_flow_data, &fn_cov_info.priority_list); let coverage_counters = transcribe_counters(&node_counters, &bcb_needs_counter, &bcbs_seen); - let mut counters_seen = DenseBitSet::new_empty(coverage_counters.node_counters.len()); - let mut expressions_seen = DenseBitSet::new_filled(coverage_counters.expressions.len()); - - // For each expression ID that is directly used by one or more mappings, - // mark it as not-yet-seen. This indicates that we expect to see a - // corresponding `VirtualCounter` statement during MIR traversal. - for mapping in fn_cov_info.mappings.iter() { - // Currently we only worry about ordinary code mappings. - // For branch and MC/DC mappings, expressions might not correspond - // to any particular point in the control-flow graph. - if let MappingKind::Code { bcb } = mapping.kind - && let Some(CovTerm::Expression(id)) = coverage_counters.node_counters[bcb] - { - expressions_seen.remove(id); - } - } - - for bcb in bcbs_seen.iter() { - if let Some(&id) = coverage_counters.phys_counter_for_node.get(&bcb) { - counters_seen.insert(id); - } - if let Some(CovTerm::Expression(id)) = coverage_counters.node_counters[bcb] { - expressions_seen.insert(id); - } - } - - let zero_expressions = identify_zero_expressions( - &coverage_counters.expressions, - &counters_seen, - &expressions_seen, - ); - - let CoverageCounters { phys_counter_for_node, node_counters, expressions, .. } = - coverage_counters; + let CoverageCounters { + phys_counter_for_node, next_counter_id, node_counters, expressions, .. + } = coverage_counters; Some(CoverageIdsInfo { - counters_seen, - zero_expressions, + num_counters: next_counter_id.as_u32(), phys_counter_for_node, term_for_bcb: node_counters, expressions, @@ -193,94 +157,3 @@ fn is_inlined(body: &Body<'_>, statement: &Statement<'_>) -> bool { let scope_data = &body.source_scopes[statement.source_info.scope]; scope_data.inlined.is_some() || scope_data.inlined_parent_scope.is_some() } - -/// Identify expressions that will always have a value of zero, and note their -/// IDs in a `DenseBitSet`. Mappings that refer to a zero expression can instead -/// become mappings to a constant zero value. -/// -/// This function mainly exists to preserve the simplifications that were -/// already being performed by the Rust-side expression renumbering, so that -/// the resulting coverage mappings don't get worse. -fn identify_zero_expressions( - expressions: &IndexSlice<ExpressionId, Expression>, - counters_seen: &DenseBitSet<CounterId>, - expressions_seen: &DenseBitSet<ExpressionId>, -) -> DenseBitSet<ExpressionId> { - // 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 `CovTerm::Zero`. - let mut zero_expressions = DenseBitSet::new_empty(expressions.len()); - - // Simplify a copy of each expression based on lower-numbered expressions, - // and then update the set of always-zero expressions if necessary. - // (By construction, expressions can only refer to other expressions - // that have lower IDs, so one pass is sufficient.) - for (id, expression) in expressions.iter_enumerated() { - if !expressions_seen.contains(id) { - // If an expression was not seen, it must have been optimized away, - // so any operand that refers to it can be replaced with zero. - zero_expressions.insert(id); - continue; - } - - // We don't need to simplify the actual expression data in the - // expressions list; we can just simplify a temporary copy and then - // use that to update the set of always-zero expressions. - let Expression { mut lhs, op, mut rhs } = *expression; - - // If an expression has an operand that is also an expression, the - // operand's ID must be strictly lower. This is what lets us find - // all zero expressions in one pass. - let assert_operand_expression_is_lower = |operand_id: ExpressionId| { - assert!( - operand_id < id, - "Operand {operand_id:?} should be less than {id:?} in {expression:?}", - ) - }; - - // If an operand refers to a counter or expression that is always - // zero, then that operand can be replaced with `CovTerm::Zero`. - let maybe_set_operand_to_zero = |operand: &mut CovTerm| { - if let CovTerm::Expression(id) = *operand { - assert_operand_expression_is_lower(id); - } - - if is_zero_term(&counters_seen, &zero_expressions, *operand) { - *operand = CovTerm::Zero; - } - }; - maybe_set_operand_to_zero(&mut lhs); - maybe_set_operand_to_zero(&mut rhs); - - // 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 lhs == CovTerm::Zero && op == Op::Subtract { - rhs = CovTerm::Zero; - } - - // After the above simplifications, if both operands are zero, then - // we know that this expression is always zero too. - if lhs == CovTerm::Zero && rhs == CovTerm::Zero { - zero_expressions.insert(id); - } - } - - zero_expressions -} - -/// Returns `true` if the given term is known to have a value of zero, taking -/// into account knowledge of which counters are unused and which expressions -/// are always zero. -fn is_zero_term( - counters_seen: &DenseBitSet<CounterId>, - zero_expressions: &DenseBitSet<ExpressionId>, - term: CovTerm, -) -> bool { - match term { - CovTerm::Zero => true, - CovTerm::Counter(id) => !counters_seen.contains(id), - CovTerm::Expression(id) => zero_expressions.contains(id), - } -} |