From 27dbb3c16a3aa1b3767c48f8fde1f48bf38a9afd Mon Sep 17 00:00:00 2001 From: Zalathar Date: Thu, 7 Sep 2023 21:08:59 +1000 Subject: [PATCH] !! (WIP) Only `CoverageKind::Mappings` can have code regions --- .../src/coverageinfo/map_data.rs | 23 +++++-------------- .../src/coverageinfo/mod.rs | 11 ++++----- compiler/rustc_middle/src/mir/coverage.rs | 9 +++++--- compiler/rustc_middle/src/mir/mod.rs | 8 ++----- compiler/rustc_middle/src/mir/syntax.rs | 3 +-- .../rustc_mir_transform/src/coverage/mod.rs | 21 ++++------------- .../rustc_mir_transform/src/coverage/query.rs | 10 ++++---- ...ument_coverage.bar.InstrumentCoverage.diff | 2 +- ...ment_coverage.main.InstrumentCoverage.diff | 8 +++---- 9 files changed, 35 insertions(+), 60 deletions(-) diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs index 1b14649f412fc..284368cee90b4 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/map_data.rs @@ -71,28 +71,20 @@ impl<'tcx> FunctionCoverage<'tcx> { } } - /// Adds code regions to be counted by an injected counter intrinsic. + /// Notes that a counter increment has been encountered with the given ID. #[instrument(level = "debug", skip(self))] - pub(crate) fn add_counter(&mut self, id: CounterId, code_regions: &[CodeRegion]) { - if self.counters_seen.insert(id) { - self.add_mappings(CovTerm::Counter(id), code_regions); - } + pub(crate) fn counter_seen(&mut self, id: CounterId) { + self.counters_seen.insert(id); } - /// Adds information about a coverage expression, along with zero or more - /// 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 `CovTerm`, so there is no ambiguity - /// between operands that are counter IDs and operands that are expression IDs. + /// Adds information about a coverage expression. #[instrument(level = "debug", skip(self))] - pub(crate) fn add_counter_expression( + pub(crate) fn add_expression_data( &mut self, expression_id: ExpressionId, lhs: CovTerm, op: Op, rhs: CovTerm, - code_regions: &[CodeRegion], ) { debug_assert!( expression_id.as_usize() < self.expressions.len(), @@ -106,10 +98,7 @@ impl<'tcx> FunctionCoverage<'tcx> { let expression = Expression { lhs, op, rhs }; let slot = &mut self.expressions[expression_id]; match slot { - None => { - *slot = Some(expression); - self.add_mappings(CovTerm::Expression(expression_id), code_regions); - } + None => *slot = Some(expression), // If this expression ID slot has already been filled, it should // contain identical information. Some(ref previous_expression) => assert_eq!( diff --git a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs index 485524a7dc44b..b494963f79c79 100644 --- a/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs +++ b/compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs @@ -110,7 +110,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { .entry(instance) .or_insert_with(|| FunctionCoverage::new(bx.tcx(), instance)); - let Coverage { kind, code_regions } = coverage; + let Coverage { kind } = coverage; match *kind { CoverageKind::Counter { function_source_hash, id } => { debug!( @@ -118,7 +118,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { instance, function_source_hash, ); func_coverage.set_function_source_hash(function_source_hash); - func_coverage.add_counter(id, code_regions); + func_coverage.counter_seen(id); // We need to explicitly drop the `RefMut` before calling into `instrprof_increment`, // as that needs an exclusive borrow. drop(coverage_map); @@ -136,12 +136,9 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> { bx.instrprof_increment(fn_name, hash, num_counters, index); } CoverageKind::Expression { id, lhs, op, rhs } => { - func_coverage.add_counter_expression(id, lhs, op, rhs, code_regions); + func_coverage.add_expression_data(id, lhs, op, rhs); } - CoverageKind::Unreachable => { - func_coverage.add_unreachable_regions(code_regions); - } - CoverageKind::Mappings { term } => { + CoverageKind::Mappings { term, ref code_regions } => { func_coverage.add_mappings(term, code_regions); } } diff --git a/compiler/rustc_middle/src/mir/coverage.rs b/compiler/rustc_middle/src/mir/coverage.rs index 77c1ef23375d3..33214da4c9cec 100644 --- a/compiler/rustc_middle/src/mir/coverage.rs +++ b/compiler/rustc_middle/src/mir/coverage.rs @@ -76,9 +76,9 @@ pub enum CoverageKind { op: Op, rhs: CovTerm, }, - Unreachable, Mappings { term: CovTerm, + code_regions: Vec, }, } @@ -98,8 +98,11 @@ impl Debug for CoverageKind { }, rhs, ), - Unreachable => write!(fmt, "Unreachable"), - Mappings { term } => fmt.debug_struct("Mappings").field("term", term).finish(), + Mappings { term, code_regions } => fmt + .debug_struct("Mappings") + .field("term", term) + .field("code_regions", code_regions) + .finish(), } } } diff --git a/compiler/rustc_middle/src/mir/mod.rs b/compiler/rustc_middle/src/mir/mod.rs index 024e548d879da..adeba94e6f976 100644 --- a/compiler/rustc_middle/src/mir/mod.rs +++ b/compiler/rustc_middle/src/mir/mod.rs @@ -1473,12 +1473,8 @@ impl Debug for Statement<'_> { AscribeUserType(box (ref place, ref c_ty), ref variance) => { write!(fmt, "AscribeUserType({place:?}, {variance:?}, {c_ty:?})") } - Coverage(box self::Coverage { ref kind, ref code_regions }) => { - if code_regions.is_empty() { - write!(fmt, "Coverage::{kind:?}") - } else { - write!(fmt, "Coverage::{kind:?} for {code_regions:?}") - } + Coverage(box self::Coverage { ref kind }) => { + write!(fmt, "Coverage::{kind:?}") } Intrinsic(box ref intrinsic) => write!(fmt, "{intrinsic}"), ConstEvalCounter => write!(fmt, "ConstEvalCounter"), diff --git a/compiler/rustc_middle/src/mir/syntax.rs b/compiler/rustc_middle/src/mir/syntax.rs index b6c9322e6319e..50e73b65a22bb 100644 --- a/compiler/rustc_middle/src/mir/syntax.rs +++ b/compiler/rustc_middle/src/mir/syntax.rs @@ -5,7 +5,7 @@ use super::{BasicBlock, Constant, Local, SwitchTargets, UserTypeProjection}; -use crate::mir::coverage::{CodeRegion, CoverageKind}; +use crate::mir::coverage::CoverageKind; use crate::traits::Reveal; use crate::ty::adjustment::PointerCoercion; use crate::ty::GenericArgsRef; @@ -523,7 +523,6 @@ pub enum FakeReadCause { #[derive(TypeFoldable, TypeVisitable)] pub struct Coverage { pub kind: CoverageKind, - pub code_regions: Vec, } #[derive(Clone, Debug, PartialEq, TyEncodable, TyDecodable, Hash, HashStable)] diff --git a/compiler/rustc_mir_transform/src/coverage/mod.rs b/compiler/rustc_mir_transform/src/coverage/mod.rs index b21334f9998a3..71afafd168f1c 100644 --- a/compiler/rustc_mir_transform/src/coverage/mod.rs +++ b/compiler/rustc_mir_transform/src/coverage/mod.rs @@ -316,15 +316,13 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { inject_statement( self.mir_body, - CoverageKind::Mappings { term: counter_kind.as_term() }, + CoverageKind::Mappings { term: counter_kind.as_term(), code_regions }, mir::START_BLOCK, - code_regions, ); inject_statement( self.mir_body, self.make_mir_coverage_kind(&counter_kind), self.bcb_leader_bb(bcb), - Vec::new(), ); } } @@ -407,7 +405,6 @@ impl<'a, 'tcx> Instrumentor<'a, 'tcx> { self.mir_body, self.make_mir_coverage_kind(&counter_kind), inject_to_bb, - Vec::new(), ); } BcbCounter::Expression { .. } => inject_intermediate_expression( @@ -473,18 +470,13 @@ fn inject_edge_counter_basic_block( new_bb } -fn inject_statement( - mir_body: &mut mir::Body<'_>, - counter_kind: CoverageKind, - bb: BasicBlock, - code_regions: Vec, -) { - debug!(" injecting statement {counter_kind:?} for {bb:?} at code regions: {code_regions:?}"); +fn inject_statement(mir_body: &mut mir::Body<'_>, counter_kind: CoverageKind, bb: BasicBlock) { + debug!(" injecting statement {counter_kind:?} for {bb:?}"); let data = &mut mir_body[bb]; let source_info = data.terminator().source_info; let statement = Statement { source_info, - kind: StatementKind::Coverage(Box::new(Coverage { kind: counter_kind, code_regions })), + kind: StatementKind::Coverage(Box::new(Coverage { kind: counter_kind })), }; data.statements.insert(0, statement); } @@ -498,10 +490,7 @@ fn inject_intermediate_expression(mir_body: &mut mir::Body<'_>, expression: Cove let source_info = data.terminator().source_info; let statement = Statement { source_info, - kind: StatementKind::Coverage(Box::new(Coverage { - kind: expression, - code_regions: Vec::new(), - })), + kind: StatementKind::Coverage(Box::new(Coverage { kind: expression })), }; data.statements.push(statement); } diff --git a/compiler/rustc_mir_transform/src/coverage/query.rs b/compiler/rustc_mir_transform/src/coverage/query.rs index 5ee119c3237b5..97f7bcbe48703 100644 --- a/compiler/rustc_mir_transform/src/coverage/query.rs +++ b/compiler/rustc_mir_transform/src/coverage/query.rs @@ -68,8 +68,7 @@ impl CoverageVisitor { self.update_from_term(lhs); self.update_from_term(rhs); } - CoverageKind::Unreachable => {} - CoverageKind::Mappings { term } => self.update_from_term(term), + CoverageKind::Mappings { term, .. } => self.update_from_term(term), } } } @@ -94,8 +93,11 @@ fn coverageinfo<'tcx>(tcx: TyCtxt<'tcx>, instance_def: ty::InstanceDef<'tcx>) -> fn covered_code_regions(tcx: TyCtxt<'_>, def_id: DefId) -> Vec<&CodeRegion> { let body = mir_body(tcx, def_id); all_coverage_in_mir_body(body) - // Coverage statements have a list of code regions (possibly empty). - .flat_map(|coverage| coverage.code_regions.as_slice()) + // Only `Mappings` statements have a list of code regions. + .flat_map(|coverage| match coverage.kind { + CoverageKind::Mappings { ref code_regions, .. } => code_regions.as_slice(), + _ => &[], + }) .collect() } diff --git a/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff b/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff index 0e7a00a26c6f2..97ee41135ccb3 100644 --- a/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff +++ b/tests/mir-opt/instrument_coverage.bar.InstrumentCoverage.diff @@ -6,7 +6,7 @@ bb0: { + Coverage::Counter(0); -+ Coverage::Mappings { term: Counter(0) } for [/the/src/instrument_coverage.rs:20:1 - 22:2]; ++ Coverage::Mappings { term: Counter(0), code_regions: [/the/src/instrument_coverage.rs:20:1 - 22:2] }; _0 = const true; return; } diff --git a/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff b/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff index 02783b736ecfb..e6c3342c491c4 100644 --- a/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff +++ b/tests/mir-opt/instrument_coverage.main.InstrumentCoverage.diff @@ -8,11 +8,11 @@ let mut _3: !; bb0: { -+ Coverage::Mappings { term: Counter(1) } for [/the/src/instrument_coverage.rs:15:10 - 15:11]; -+ Coverage::Mappings { term: Expression(1) } for [/the/src/instrument_coverage.rs:14:13 - 14:18, /the/src/instrument_coverage.rs:17:1 - 17:2]; -+ Coverage::Mappings { term: Expression(0) } for [/the/src/instrument_coverage.rs:12:5 - 13:17]; ++ Coverage::Mappings { term: Counter(1), code_regions: [/the/src/instrument_coverage.rs:15:10 - 15:11] }; ++ Coverage::Mappings { term: Expression(1), code_regions: [/the/src/instrument_coverage.rs:14:13 - 14:18, /the/src/instrument_coverage.rs:17:1 - 17:2] }; ++ Coverage::Mappings { term: Expression(0), code_regions: [/the/src/instrument_coverage.rs:12:5 - 13:17] }; + Coverage::Counter(0); -+ Coverage::Mappings { term: Counter(0) } for [/the/src/instrument_coverage.rs:11:1 - 11:11]; ++ Coverage::Mappings { term: Counter(0), code_regions: [/the/src/instrument_coverage.rs:11:1 - 11:11] }; goto -> bb1; }