From 8c7c84b4e8923779ff56a518e4cd39c1c600c7d0 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Thu, 18 Jun 2020 13:29:43 -0700 Subject: code coverage foundation for hash and num_counters Replaced dummy values for hash and num_counters with computed values, and refactored InstrumentCoverage pass to simplify injecting more counters per function in upcoming versions. Improved usage documentation and error messaging. --- src/librustc_codegen_llvm/intrinsic.rs | 37 +++++++++++++--------------------- 1 file changed, 14 insertions(+), 23 deletions(-) (limited to 'src/librustc_codegen_llvm') diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 95465939070..8c0ccde6b16 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -16,6 +16,7 @@ use rustc_codegen_ssa::common::TypeKind; use rustc_codegen_ssa::glue; use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue}; use rustc_codegen_ssa::mir::place::PlaceRef; +use rustc_codegen_ssa::mir::FunctionCx; use rustc_codegen_ssa::traits::*; use rustc_codegen_ssa::MemFlags; use rustc_hir as hir; @@ -23,7 +24,6 @@ use rustc_middle::ty::layout::{FnAbiExt, HasTyCtxt}; use rustc_middle::ty::{self, Ty}; use rustc_middle::{bug, span_bug}; use rustc_span::Span; -use rustc_span::Symbol; use rustc_target::abi::{self, HasDataLayout, LayoutOf, Primitive}; use rustc_target::spec::PanicStrategy; @@ -82,14 +82,14 @@ fn get_simple_intrinsic(cx: &CodegenCx<'ll, '_>, name: &str) -> Option<&'ll Valu } impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { - fn codegen_intrinsic_call( + fn codegen_intrinsic_call<'b, Bx: BuilderMethods<'b, 'tcx>>( &mut self, + fx: &FunctionCx<'b, 'tcx, Bx>, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, args: &[OperandRef<'tcx, &'ll Value>], llresult: &'ll Value, span: Span, - caller_instance: ty::Instance<'tcx>, ) { let tcx = self.tcx; let callee_ty = instance.monomorphic_ty(tcx); @@ -141,26 +141,17 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { self.call(llfn, &[], None) } "count_code_region" => { - if let ty::InstanceDef::Item(fn_def_id) = caller_instance.def { - let caller_fn_path = tcx.def_path_str(fn_def_id); - debug!( - "count_code_region to llvm.instrprof.increment(fn_name={})", - caller_fn_path - ); - - // FIXME(richkadel): (1) Replace raw function name with mangled function name; - // (2) Replace hardcoded `1234` in `hash` with a computed hash (as discussed in) - // the MCP (compiler-team/issues/278); and replace the hardcoded `1` for - // `num_counters` with the actual number of counters per function (when the - // changes are made to inject more than one counter per function). - let (fn_name, _len_val) = self.const_str(Symbol::intern(&caller_fn_path)); - let index = args[0].immediate(); - let hash = self.const_u64(1234); - let num_counters = self.const_u32(1); - self.instrprof_increment(fn_name, hash, num_counters, index) - } else { - bug!("intrinsic count_code_region: no src.instance"); - } + let coverage_data = fx.mir.coverage_data.as_ref().unwrap(); + let mangled_fn = tcx.symbol_name(fx.instance); + let (mangled_fn_name, _len_val) = self.const_str(mangled_fn.name); + let hash = self.const_u64(coverage_data.hash); + let index = args[0].immediate(); + let num_counters = self.const_u32(coverage_data.num_counters as u32); + debug!( + "count_code_region to LLVM intrinsic instrprof.increment(fn_name={}, hash={:?}, num_counters={:?}, index={:?})", + mangled_fn.name, hash, index, num_counters + ); + self.instrprof_increment(mangled_fn_name, hash, num_counters, index) } "va_start" => self.va_start(args[0].immediate()), "va_end" => self.va_end(args[0].immediate()), -- cgit 1.4.1-3-g733a5 From 933fe805777e46163c52371d81390ba721a37252 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Sun, 21 Jun 2020 23:48:39 -0700 Subject: num_counters to u32, after implementing TypeFoldable --- src/librustc_codegen_llvm/intrinsic.rs | 2 +- src/librustc_middle/mir/mod.rs | 2 +- src/librustc_middle/ty/structural_impls.rs | 1 + src/librustc_mir/transform/instrument_coverage.rs | 6 +++--- 4 files changed, 6 insertions(+), 5 deletions(-) (limited to 'src/librustc_codegen_llvm') diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index 8c0ccde6b16..c6e7820a60e 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -146,7 +146,7 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { let (mangled_fn_name, _len_val) = self.const_str(mangled_fn.name); let hash = self.const_u64(coverage_data.hash); let index = args[0].immediate(); - let num_counters = self.const_u32(coverage_data.num_counters as u32); + let num_counters = self.const_u32(coverage_data.num_counters); debug!( "count_code_region to LLVM intrinsic instrprof.increment(fn_name={}, hash={:?}, num_counters={:?}, index={:?})", mangled_fn.name, hash, index, num_counters diff --git a/src/librustc_middle/mir/mod.rs b/src/librustc_middle/mir/mod.rs index a89a5ef3f82..e88329db992 100644 --- a/src/librustc_middle/mir/mod.rs +++ b/src/librustc_middle/mir/mod.rs @@ -98,7 +98,7 @@ pub struct CoverageData { pub hash: u64, /// The total number of coverage region counters added to this MIR Body. - pub num_counters: usize, + pub num_counters: u32, } /// The lowered representation of a single function. diff --git a/src/librustc_middle/ty/structural_impls.rs b/src/librustc_middle/ty/structural_impls.rs index f04d31601ea..463e2c57d46 100644 --- a/src/librustc_middle/ty/structural_impls.rs +++ b/src/librustc_middle/ty/structural_impls.rs @@ -262,6 +262,7 @@ CloneTypeFoldableAndLiftImpls! { bool, usize, ::rustc_target::abi::VariantIdx, + u32, u64, String, crate::middle::region::Scope, diff --git a/src/librustc_mir/transform/instrument_coverage.rs b/src/librustc_mir/transform/instrument_coverage.rs index 793ccbb081b..a24d0acf421 100644 --- a/src/librustc_mir/transform/instrument_coverage.rs +++ b/src/librustc_mir/transform/instrument_coverage.rs @@ -21,7 +21,7 @@ pub struct InstrumentCoverage; struct Instrumentor<'tcx> { tcx: TyCtxt<'tcx>, - num_counters: usize, + num_counters: u32, } impl<'tcx> MirPass<'tcx> for InstrumentCoverage { @@ -55,12 +55,12 @@ impl<'tcx> Instrumentor<'tcx> { } fn next_counter(&mut self) -> u32 { - let next = self.num_counters as u32; + let next = self.num_counters; self.num_counters += 1; next } - fn inject_counters(&mut self, mir_body: &mut mir::Body<'tcx>) -> usize { + fn inject_counters(&mut self, mir_body: &mut mir::Body<'tcx>) -> u32 { // FIXME(richkadel): As a first step, counters are only injected at the top of each // function. The complete solution will inject counters at each conditional code branch. let top_of_function = START_BLOCK; -- cgit 1.4.1-3-g733a5 From f4a79385cf28b263894be9ebd2e541532ae82898 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 22 Jun 2020 14:11:55 -0700 Subject: implemented query for coverage data This commit adds a query that allows the CoverageData to be pulled from a call on tcx, avoiding the need to change the `codegen_intrinsic_call()` signature (no need to pass in the FunctionCx or any additional arguments. The commit does not change where/when the CoverageData is computed. It's still done in the `pass`, and saved in the MIR `Body`. See discussion (in progress) here: https://github.com/rust-lang/rust/pull/73488#discussion_r443825646 --- src/librustc_codegen_llvm/intrinsic.rs | 12 +++++++----- src/librustc_codegen_ssa/mir/block.rs | 2 +- src/librustc_codegen_ssa/mir/mod.rs | 4 ++-- src/librustc_codegen_ssa/traits/intrinsic.rs | 6 ++---- src/librustc_middle/query/mod.rs | 6 ++++++ src/librustc_mir/transform/mod.rs | 8 +++++++- 6 files changed, 25 insertions(+), 13 deletions(-) (limited to 'src/librustc_codegen_llvm') diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index c6e7820a60e..f1104ca3a98 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -16,7 +16,6 @@ use rustc_codegen_ssa::common::TypeKind; use rustc_codegen_ssa::glue; use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue}; use rustc_codegen_ssa::mir::place::PlaceRef; -use rustc_codegen_ssa::mir::FunctionCx; use rustc_codegen_ssa::traits::*; use rustc_codegen_ssa::MemFlags; use rustc_hir as hir; @@ -82,14 +81,14 @@ fn get_simple_intrinsic(cx: &CodegenCx<'ll, '_>, name: &str) -> Option<&'ll Valu } impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { - fn codegen_intrinsic_call<'b, Bx: BuilderMethods<'b, 'tcx>>( + fn codegen_intrinsic_call( &mut self, - fx: &FunctionCx<'b, 'tcx, Bx>, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, args: &[OperandRef<'tcx, &'ll Value>], llresult: &'ll Value, span: Span, + caller_instance: ty::Instance<'tcx>, ) { let tcx = self.tcx; let callee_ty = instance.monomorphic_ty(tcx); @@ -141,8 +140,11 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { self.call(llfn, &[], None) } "count_code_region" => { - let coverage_data = fx.mir.coverage_data.as_ref().unwrap(); - let mangled_fn = tcx.symbol_name(fx.instance); + let coverage_data = tcx + .coverage_data(caller_instance.def_id()) + .as_ref() + .expect("LLVM intrinsic count_code_region call has associated coverage_data"); + let mangled_fn = tcx.symbol_name(caller_instance); let (mangled_fn_name, _len_val) = self.const_str(mangled_fn.name); let hash = self.const_u64(coverage_data.hash); let index = args[0].immediate(); diff --git a/src/librustc_codegen_ssa/mir/block.rs b/src/librustc_codegen_ssa/mir/block.rs index 945c3d48434..d56c816811b 100644 --- a/src/librustc_codegen_ssa/mir/block.rs +++ b/src/librustc_codegen_ssa/mir/block.rs @@ -688,12 +688,12 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { .collect(); bx.codegen_intrinsic_call( - self, *instance.as_ref().unwrap(), &fn_abi, &args, dest, terminator.source_info.span, + self.instance, ); if let ReturnDest::IndirectOperand(dst, _) = ret_dest { diff --git a/src/librustc_codegen_ssa/mir/mod.rs b/src/librustc_codegen_ssa/mir/mod.rs index fd2e779f681..00b4bf96afa 100644 --- a/src/librustc_codegen_ssa/mir/mod.rs +++ b/src/librustc_codegen_ssa/mir/mod.rs @@ -21,9 +21,9 @@ use self::operand::{OperandRef, OperandValue}; /// Master context for codegenning from MIR. pub struct FunctionCx<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> { - pub instance: Instance<'tcx>, + instance: Instance<'tcx>, - pub mir: &'tcx mir::Body<'tcx>, + mir: &'tcx mir::Body<'tcx>, debug_context: Option>, diff --git a/src/librustc_codegen_ssa/traits/intrinsic.rs b/src/librustc_codegen_ssa/traits/intrinsic.rs index 0036eadf4db..f6201949851 100644 --- a/src/librustc_codegen_ssa/traits/intrinsic.rs +++ b/src/librustc_codegen_ssa/traits/intrinsic.rs @@ -1,7 +1,5 @@ use super::BackendTypes; use crate::mir::operand::OperandRef; -use crate::mir::FunctionCx; -use crate::traits::BuilderMethods; use rustc_middle::ty::{self, Ty}; use rustc_span::Span; use rustc_target::abi::call::FnAbi; @@ -10,14 +8,14 @@ pub trait IntrinsicCallMethods<'tcx>: BackendTypes { /// Remember to add all intrinsics here, in librustc_typeck/check/mod.rs, /// and in libcore/intrinsics.rs; if you need access to any llvm intrinsics, /// add them to librustc_codegen_llvm/context.rs - fn codegen_intrinsic_call<'a, Bx: BuilderMethods<'a, 'tcx>>( + fn codegen_intrinsic_call( &mut self, - fx: &FunctionCx<'a, 'tcx, Bx>, instance: ty::Instance<'tcx>, fn_abi: &FnAbi<'tcx, Ty<'tcx>>, args: &[OperandRef<'tcx, Self::Value>], llresult: Self::Value, span: Span, + caller_instance: ty::Instance<'tcx>, ); fn abort(&mut self); diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index 3b6d54a1bc1..1092e6c3064 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -214,6 +214,12 @@ rustc_queries! { cache_on_disk_if { key.is_local() } } + query coverage_data(key: DefId) -> Option { + desc { |tcx| "retrieving coverage data, if computed from MIR for `{}`", tcx.def_path_str(key) } + storage(ArenaCacheSelector<'tcx>) + cache_on_disk_if { key.is_local() } + } + query promoted_mir(key: DefId) -> IndexVec> { desc { |tcx| "optimizing promoted MIR for `{}`", tcx.def_path_str(key) } storage(ArenaCacheSelector<'tcx>) diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index 846ed1f86d8..a2fdc7efd18 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -6,7 +6,7 @@ use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE}; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_index::vec::IndexVec; use rustc_middle::mir::visit::Visitor as _; -use rustc_middle::mir::{traversal, Body, ConstQualifs, MirPhase, Promoted}; +use rustc_middle::mir::{traversal, Body, ConstQualifs, CoverageData, MirPhase, Promoted}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::steal::Steal; use rustc_middle::ty::{InstanceDef, TyCtxt, TypeFoldable}; @@ -53,6 +53,7 @@ pub(crate) fn provide(providers: &mut Providers<'_>) { mir_drops_elaborated_and_const_checked, optimized_mir, is_mir_available, + coverage_data, promoted_mir, ..*providers }; @@ -422,6 +423,11 @@ fn run_optimization_passes<'tcx>( ); } +fn coverage_data(tcx: TyCtxt<'_>, def_id: DefId) -> Option { + let body = tcx.optimized_mir(def_id); + body.coverage_data.clone() +} + fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> Body<'_> { if tcx.is_constructor(def_id) { // There's no reason to run all of the MIR passes on constructors when -- cgit 1.4.1-3-g733a5 From 08ec4cbb9e0672118946c85f410b50ea4001b1dd Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 22 Jun 2020 19:21:56 -0700 Subject: moves coverage data computation from pass to query --- src/librustc_codegen_llvm/intrinsic.rs | 9 ++-- src/librustc_middle/mir/mod.rs | 34 ++++++------- src/librustc_middle/query/mod.rs | 2 +- src/librustc_mir/transform/instrument_coverage.rs | 58 ++++++++++++++--------- src/librustc_mir/transform/mod.rs | 9 +--- 5 files changed, 56 insertions(+), 56 deletions(-) (limited to 'src/librustc_codegen_llvm') diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index f1104ca3a98..b9193a85b1e 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -140,18 +140,15 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { self.call(llfn, &[], None) } "count_code_region" => { - let coverage_data = tcx - .coverage_data(caller_instance.def_id()) - .as_ref() - .expect("LLVM intrinsic count_code_region call has associated coverage_data"); + let coverage_data = tcx.coverage_data(caller_instance.def_id()); let mangled_fn = tcx.symbol_name(caller_instance); let (mangled_fn_name, _len_val) = self.const_str(mangled_fn.name); let hash = self.const_u64(coverage_data.hash); - let index = args[0].immediate(); let num_counters = self.const_u32(coverage_data.num_counters); + let index = args[0].immediate(); debug!( "count_code_region to LLVM intrinsic instrprof.increment(fn_name={}, hash={:?}, num_counters={:?}, index={:?})", - mangled_fn.name, hash, index, num_counters + mangled_fn.name, hash, num_counters, index ); self.instrprof_increment(mangled_fn_name, hash, num_counters, index) } diff --git a/src/librustc_middle/mir/mod.rs b/src/librustc_middle/mir/mod.rs index e88329db992..854fda095b6 100644 --- a/src/librustc_middle/mir/mod.rs +++ b/src/librustc_middle/mir/mod.rs @@ -88,19 +88,6 @@ impl MirPhase { } } -/// Coverage data computed by the `InstrumentCoverage` MIR pass, when compiling with -/// `-Zinstrument_coverage`. -#[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable, TypeFoldable)] -pub struct CoverageData { - /// A hash value that can be used by the consumer of the coverage profile data to detect - /// changes to the instrumented source of the associated MIR body (typically, for an - /// individual function). - pub hash: u64, - - /// The total number of coverage region counters added to this MIR Body. - pub num_counters: u32, -} - /// The lowered representation of a single function. #[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable, TypeFoldable)] pub struct Body<'tcx> { @@ -184,10 +171,6 @@ pub struct Body<'tcx> { /// FIXME(oli-obk): rewrite the promoted during promotion to eliminate the cell components. pub ignore_interior_mut_in_const_validation: bool, - /// If compiling with `-Zinstrument_coverage`, the `InstrumentCoverage` pass stores summary - /// information associated with the MIR, used in code generation of the coverage counters. - pub coverage_data: Option, - predecessor_cache: PredecessorCache, } @@ -228,7 +211,6 @@ impl<'tcx> Body<'tcx> { required_consts: Vec::new(), ignore_interior_mut_in_const_validation: false, control_flow_destroyed, - coverage_data: None, predecessor_cache: PredecessorCache::new(), } } @@ -256,7 +238,6 @@ impl<'tcx> Body<'tcx> { generator_kind: None, var_debug_info: Vec::new(), ignore_interior_mut_in_const_validation: false, - coverage_data: None, predecessor_cache: PredecessorCache::new(), } } @@ -2938,3 +2919,18 @@ impl Location { } } } + +/// Coverage data associated with each function (MIR) instrumented with coverage counters, when +/// compiled with `-Zinstrument_coverage`. The query `tcx.coverage_data(DefId)` computes these +/// values on demand (during code generation). This query is only valid after executing the MIR pass +/// `InstrumentCoverage`. +#[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)] +pub struct CoverageData { + /// A hash value that can be used by the consumer of the coverage profile data to detect + /// changes to the instrumented source of the associated MIR body (typically, for an + /// individual function). + pub hash: u64, + + /// The total number of coverage region counters added to the MIR `Body`. + pub num_counters: u32, +} diff --git a/src/librustc_middle/query/mod.rs b/src/librustc_middle/query/mod.rs index 993b48afb7a..4815f2617b6 100644 --- a/src/librustc_middle/query/mod.rs +++ b/src/librustc_middle/query/mod.rs @@ -214,7 +214,7 @@ rustc_queries! { cache_on_disk_if { key.is_local() } } - query coverage_data(key: DefId) -> Option { + query coverage_data(key: DefId) -> mir::CoverageData { desc { |tcx| "retrieving coverage data from MIR for `{}`", tcx.def_path_str(key) } storage(ArenaCacheSelector<'tcx>) cache_on_disk_if { key.is_local() } diff --git a/src/librustc_mir/transform/instrument_coverage.rs b/src/librustc_mir/transform/instrument_coverage.rs index 94aa26b3081..8d263c3089c 100644 --- a/src/librustc_mir/transform/instrument_coverage.rs +++ b/src/librustc_mir/transform/instrument_coverage.rs @@ -7,10 +7,12 @@ use rustc_middle::hir; use rustc_middle::ich::StableHashingContext; use rustc_middle::mir::interpret::Scalar; use rustc_middle::mir::{ - self, BasicBlock, BasicBlockData, CoverageData, Operand, Place, SourceInfo, StatementKind, - Terminator, TerminatorKind, START_BLOCK, + self, traversal, BasicBlock, BasicBlockData, CoverageData, Operand, Place, SourceInfo, + StatementKind, Terminator, TerminatorKind, START_BLOCK, }; use rustc_middle::ty; +use rustc_middle::ty::query::Providers; +use rustc_middle::ty::FnDef; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::DefId; use rustc_span::Span; @@ -19,6 +21,31 @@ use rustc_span::Span; /// the intrinsic llvm.instrprof.increment. pub struct InstrumentCoverage; +/// The `query` provider for `CoverageData`, requested by `codegen_intrinsic_call()` when +/// constructing the arguments for `llvm.instrprof.increment`. +pub(crate) fn provide(providers: &mut Providers<'_>) { + providers.coverage_data = |tcx, def_id| { + let body = tcx.optimized_mir(def_id); + let count_code_region_fn = + tcx.require_lang_item(lang_items::CountCodeRegionFnLangItem, None); + let mut num_counters: u32 = 0; + for (_, data) in traversal::preorder(body) { + if let Some(terminator) = &data.terminator { + if let TerminatorKind::Call { func: Operand::Constant(func), .. } = &terminator.kind + { + if let FnDef(called_fn_def_id, _) = func.literal.ty.kind { + if called_fn_def_id == count_code_region_fn { + num_counters += 1; + } + } + } + } + } + let hash = if num_counters > 0 { hash_mir_source(tcx, def_id) } else { 0 }; + CoverageData { num_counters, hash } + }; +} + struct Instrumentor<'tcx> { tcx: TyCtxt<'tcx>, num_counters: u32, @@ -30,20 +57,12 @@ impl<'tcx> MirPass<'tcx> for InstrumentCoverage { // If the InstrumentCoverage pass is called on promoted MIRs, skip them. // See: https://github.com/rust-lang/rust/pull/73011#discussion_r438317601 if src.promoted.is_none() { - assert!(mir_body.coverage_data.is_none()); - - let hash = hash_mir_source(tcx, &src); - debug!( - "instrumenting {:?}, hash: {}, span: {}", + "instrumenting {:?}, span: {}", src.def_id(), - hash, tcx.sess.source_map().span_to_string(mir_body.span) ); - - let num_counters = Instrumentor::new(tcx).inject_counters(mir_body); - - mir_body.coverage_data = Some(CoverageData { hash, num_counters }); + Instrumentor::new(tcx).inject_counters(mir_body); } } } @@ -60,15 +79,13 @@ impl<'tcx> Instrumentor<'tcx> { next } - fn inject_counters(&mut self, mir_body: &mut mir::Body<'tcx>) -> u32 { + fn inject_counters(&mut self, mir_body: &mut mir::Body<'tcx>) { // FIXME(richkadel): As a first step, counters are only injected at the top of each // function. The complete solution will inject counters at each conditional code branch. let top_of_function = START_BLOCK; let entire_function = mir_body.span; self.inject_counter(mir_body, top_of_function, entire_function); - - self.num_counters } fn inject_counter( @@ -138,14 +155,9 @@ fn placeholder_block(span: Span) -> BasicBlockData<'tcx> { } } -fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, src: &MirSource<'tcx>) -> u64 { - let fn_body_id = match tcx.hir().get_if_local(src.def_id()) { - Some(node) => match hir::map::associated_body(node) { - Some(body_id) => body_id, - _ => bug!("instrumented MirSource does not include a function body: {:?}", node), - }, - None => bug!("instrumented MirSource is not local: {:?}", src), - }; +fn hash_mir_source<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId) -> u64 { + let hir_node = tcx.hir().get_if_local(def_id).expect("DefId is local"); + let fn_body_id = hir::map::associated_body(hir_node).expect("HIR node is a function with body"); let hir_body = tcx.hir().body(fn_body_id); let mut hcx = tcx.create_no_span_stable_hashing_context(); hash(&mut hcx, &hir_body.value).to_smaller_hash() diff --git a/src/librustc_mir/transform/mod.rs b/src/librustc_mir/transform/mod.rs index a2fdc7efd18..8ca240d2c7d 100644 --- a/src/librustc_mir/transform/mod.rs +++ b/src/librustc_mir/transform/mod.rs @@ -6,7 +6,7 @@ use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE}; use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_index::vec::IndexVec; use rustc_middle::mir::visit::Visitor as _; -use rustc_middle::mir::{traversal, Body, ConstQualifs, CoverageData, MirPhase, Promoted}; +use rustc_middle::mir::{traversal, Body, ConstQualifs, MirPhase, Promoted}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::steal::Steal; use rustc_middle::ty::{InstanceDef, TyCtxt, TypeFoldable}; @@ -53,10 +53,10 @@ pub(crate) fn provide(providers: &mut Providers<'_>) { mir_drops_elaborated_and_const_checked, optimized_mir, is_mir_available, - coverage_data, promoted_mir, ..*providers }; + instrument_coverage::provide(providers); } fn is_mir_available(tcx: TyCtxt<'_>, def_id: DefId) -> bool { @@ -423,11 +423,6 @@ fn run_optimization_passes<'tcx>( ); } -fn coverage_data(tcx: TyCtxt<'_>, def_id: DefId) -> Option { - let body = tcx.optimized_mir(def_id); - body.coverage_data.clone() -} - fn optimized_mir(tcx: TyCtxt<'_>, def_id: DefId) -> Body<'_> { if tcx.is_constructor(def_id) { // There's no reason to run all of the MIR passes on constructors when -- cgit 1.4.1-3-g733a5 From 977ce57d915914139c4aa643e63f368913e5f437 Mon Sep 17 00:00:00 2001 From: Rich Kadel Date: Mon, 22 Jun 2020 23:31:41 -0700 Subject: Updated query for num_counters to compute from max index Also added FIXME comments to note the possible need to accommodate counter increment calls in source-based functions that differ from the function context of the caller instance (e.g., inline functions). --- src/librustc_codegen_llvm/intrinsic.rs | 3 +++ src/librustc_mir/transform/instrument_coverage.rs | 28 +++++++++++++++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) (limited to 'src/librustc_codegen_llvm') diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs index b9193a85b1e..dfe97b1ee2e 100644 --- a/src/librustc_codegen_llvm/intrinsic.rs +++ b/src/librustc_codegen_llvm/intrinsic.rs @@ -140,6 +140,9 @@ impl IntrinsicCallMethods<'tcx> for Builder<'a, 'll, 'tcx> { self.call(llfn, &[], None) } "count_code_region" => { + // FIXME(richkadel): The current implementation assumes the MIR for the given + // caller_instance represents a single function. Validate and/or correct if inlining + // and/or monomorphization invalidates these assumptions. let coverage_data = tcx.coverage_data(caller_instance.def_id()); let mangled_fn = tcx.symbol_name(caller_instance); let (mangled_fn_name, _len_val) = self.const_str(mangled_fn.name); diff --git a/src/librustc_mir/transform/instrument_coverage.rs b/src/librustc_mir/transform/instrument_coverage.rs index 27aaf47bbf2..06b648ab5a9 100644 --- a/src/librustc_mir/transform/instrument_coverage.rs +++ b/src/librustc_mir/transform/instrument_coverage.rs @@ -5,15 +5,15 @@ use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; use rustc_hir::lang_items; use rustc_middle::hir; use rustc_middle::ich::StableHashingContext; -use rustc_middle::mir::interpret::Scalar; +use rustc_middle::mir::interpret::{ConstValue, Scalar}; use rustc_middle::mir::{ self, traversal, BasicBlock, BasicBlockData, CoverageData, Operand, Place, SourceInfo, StatementKind, Terminator, TerminatorKind, START_BLOCK, }; use rustc_middle::ty; use rustc_middle::ty::query::Providers; -use rustc_middle::ty::FnDef; use rustc_middle::ty::TyCtxt; +use rustc_middle::ty::{ConstKind, FnDef}; use rustc_span::def_id::DefId; use rustc_span::Span; @@ -26,16 +26,36 @@ pub struct InstrumentCoverage; pub(crate) fn provide(providers: &mut Providers<'_>) { providers.coverage_data = |tcx, def_id| { let mir_body = tcx.optimized_mir(def_id); + // FIXME(richkadel): The current implementation assumes the MIR for the given DefId + // represents a single function. Validate and/or correct if inlining and/or monomorphization + // invalidates these assumptions. let count_code_region_fn = tcx.require_lang_item(lang_items::CountCodeRegionFnLangItem, None); let mut num_counters: u32 = 0; + // The `num_counters` argument to `llvm.instrprof.increment` is the number of injected + // counters, with each counter having an index from `0..num_counters-1`. MIR optimization + // may split and duplicate some BasicBlock sequences. Simply counting the calls may not + // not work; but computing the num_counters by adding `1` to the highest index (for a given + // instrumented function) is valid. for (_, data) in traversal::preorder(mir_body) { if let Some(terminator) = &data.terminator { - if let TerminatorKind::Call { func: Operand::Constant(func), .. } = &terminator.kind + if let TerminatorKind::Call { func: Operand::Constant(func), args, .. } = + &terminator.kind { if let FnDef(called_fn_def_id, _) = func.literal.ty.kind { if called_fn_def_id == count_code_region_fn { - num_counters += 1; + if let Operand::Constant(constant) = + args.get(0).expect("count_code_region has at least one arg") + { + if let ConstKind::Value(ConstValue::Scalar(value)) = + constant.literal.val + { + let index = value + .to_u32() + .expect("count_code_region index at arg0 is u32"); + num_counters = std::cmp::max(num_counters, index + 1); + } + } } } } -- cgit 1.4.1-3-g733a5