mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Addressed all feedback to date

Browse Source
This commit is contained in:
Rich Kadel 2020-10-25 11:13:16 -07:00
parent 5545c56e9d
commit 1973f84ebb
8 changed files with 41 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
View File

@ -82,21 +82,19 @@ fn set_function_source_hash(
fn add_coverage_counter(
&mut self,
instance: Instance<'tcx>,
function_source_hash: u64,
id: CounterValueReference,
region: CodeRegion,
) -> bool {
if let Some(coverage_context) = self.coverage_context() {
debug!(
"adding counter to coverage_map: instance={:?}, function_source_hash={}, id={:?}, \
at {:?}",
instance, function_source_hash, id, region,
"adding counter to coverage_map: instance={:?}, id={:?}, region={:?}",
instance, id, region,
);
let mut coverage_map = coverage_context.function_coverage_map.borrow_mut();
coverage_map
.entry(instance)
.or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
.add_counter(function_source_hash, id, region);
.add_counter(id, region);
true
} else {
false

16
compiler/rustc_codegen_ssa/src/coverageinfo/map.rs
View File

@ -52,11 +52,8 @@ pub fn new(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) -> Self {
}
}
/// Although every function should have at least one `Counter`, the `Counter` isn't required to
/// have a `CodeRegion`. (The `CodeRegion` may be associated only with `Expressions`.) This
/// method supports the ability to ensure the `function_source_hash` is set from `Counters` that
/// do not trigger the call to `add_counter()` because they don't have an associated
/// `CodeRegion` to add.
/// Sets the function source hash value. If called multiple times for the same function, all
/// calls should have the same hash value.
pub fn set_function_source_hash(&mut self, source_hash: u64) {
if self.source_hash == 0 {
self.source_hash = source_hash;
@ -66,14 +63,7 @@ pub fn set_function_source_hash(&mut self, source_hash: u64) {
}
/// Adds a code region to be counted by an injected counter intrinsic.
/// The source_hash (computed during coverage instrumentation) should also be provided, and
/// should be the same for all counters in a given function.
pub fn add_counter(&mut self, source_hash: u64, id: CounterValueReference, region: CodeRegion) {
if self.source_hash == 0 {
self.source_hash = source_hash;
} else {
debug_assert_eq!(source_hash, self.source_hash);
}
pub fn add_counter(&mut self, id: CounterValueReference, region: CodeRegion) {
self.counters[id].replace(region).expect_none("add_counter called with duplicate `id`");
}

17
compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs
View File

@ -10,15 +10,16 @@ pub fn codegen_coverage(&self, bx: &mut Bx, coverage: Coverage) {
let Coverage { kind, code_region } = coverage;
match kind {
CoverageKind::Counter { function_source_hash, id } => {
let covmap_updated = if let Some(code_region) = code_region {
// Note: Some counters do not have code regions, but may still be referenced from
// expressions.
bx.add_coverage_counter(self.instance, function_source_hash, id, code_region)
} else {
bx.set_function_source_hash(self.instance, function_source_hash)
};
if bx.set_function_source_hash(self.instance, function_source_hash) {
// If `set_function_source_hash()` returned true, the coverage map is enabled,
// so continue adding the counter.
if let Some(code_region) = code_region {
// Note: Some counters do not have code regions, but may still be referenced
// from expressions. In that case, don't add the counter to the coverage map,
// but do inject the counter intrinsic.
bx.add_coverage_counter(self.instance, id, code_region);
}
if covmap_updated {
let coverageinfo = bx.tcx().coverageinfo(self.instance.def_id());
let fn_name = bx.create_pgo_func_name_var(self.instance);

6
compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs
View File

@ -9,8 +9,9 @@ pub trait CoverageInfoMethods: BackendTypes {
pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes {
fn create_pgo_func_name_var(&self, instance: Instance<'tcx>) -> Self::Value;
/// Returns true if the function source hash was added to the coverage map; false if
/// `-Z instrument-coverage` is not enabled (a coverage map is not being generated).
/// Returns true if the function source hash was added to the coverage map (even if it had
/// already been added, for this instance). Returns false *only* if `-Z instrument-coverage` is
/// not enabled (a coverage map is not being generated).
fn set_function_source_hash(
&mut self,
instance: Instance<'tcx>,
@ -22,7 +23,6 @@ fn set_function_source_hash(
fn add_coverage_counter(
&mut self,
instance: Instance<'tcx>,
function_source_hash: u64,
index: CounterValueReference,
region: CodeRegion,
) -> bool;

14
compiler/rustc_middle/src/mir/coverage.rs
View File

@ -85,13 +85,6 @@ fn from(v: CounterValueReference) -> ExpressionOperandId {
}
}
impl From<&mut CounterValueReference> for ExpressionOperandId {
#[inline]
fn from(v: &mut CounterValueReference) -> ExpressionOperandId {
ExpressionOperandId::from(v.as_u32())
}
}
impl From<InjectedExpressionId> for ExpressionOperandId {
#[inline]
fn from(v: InjectedExpressionId) -> ExpressionOperandId {
@ -99,13 +92,6 @@ fn from(v: InjectedExpressionId) -> ExpressionOperandId {
}
}
impl From<&mut InjectedExpressionId> for ExpressionOperandId {
#[inline]
fn from(v: &mut InjectedExpressionId) -> ExpressionOperandId {
ExpressionOperandId::from(v.as_u32())
}
}
#[derive(Clone, PartialEq, TyEncodable, TyDecodable, HashStable, TypeFoldable)]
pub enum CoverageKind {
Counter {

11
compiler/rustc_mir/src/transform/coverage/counters.rs
View File

@ -12,6 +12,16 @@
use rustc_index::bit_set::BitSet;
use rustc_middle::mir::coverage::*;
// When evaluating an expression operand to determine if it references a `Counter` or an
// `Expression`, the range of counter or expression IDs must be known in order to answer the
// question: "Does this ID fall inside the range of counters," for example. If "yes," the ID refers
// to a counter, otherwise the ID refers to an expression.
//
// But in situations where the range is not currently known, the only fallback is to assume a
// specific range limit. `MAX_COUNTER_GUARD` enforces a limit on the number of counters, and
// therefore a limit on the range of counter IDs.
pub(crate) const MAX_COUNTER_GUARD: u32 = (u32::MAX / 2) + 1;
/// Manages the counter and expression indexes/IDs to generate `CoverageKind` components for MIR
/// `Coverage` statements.
pub(crate) struct CoverageCounters {
@ -95,6 +105,7 @@ pub fn make_identity_counter(&mut self, counter_operand: ExpressionOperandId) ->
/// Counter IDs start from one and go up.
fn next_counter(&mut self) -> CounterValueReference {
assert!(self.next_counter_id < u32::MAX - self.num_expressions);
assert!(self.next_counter_id <= MAX_COUNTER_GUARD);
let next = self.next_counter_id;
self.next_counter_id += 1;
CounterValueReference::from(next)

20
compiler/rustc_mir/src/transform/coverage/query.rs
View File

@ -1,3 +1,5 @@
use super::counters;
use rustc_middle::mir::coverage::*;
use rustc_middle::mir::visit::Visitor;
use rustc_middle::mir::{Coverage, CoverageInfo, Location};
@ -32,21 +34,16 @@ pub(crate) fn provide(providers: &mut Providers) {
/// safeguard, with `add_missing_operands` set to `true`, to find any other counter or expression
/// IDs referenced by expression operands, if not already seen.
///
/// Ideally, every expression operand in the MIR will have a corresponding Counter or Expression,
/// but since current or future MIR optimizations can theoretically optimize out segments of a
/// MIR, it may not be possible to guarantee this, so the second pass ensures the `CoverageInfo`
/// counts include all referenced IDs.
/// Ideally, each operand ID in a MIR `CoverageKind::Expression` will have a separate MIR `Coverage`
/// statement for the `Counter` or `Expression` with the referenced ID. but since current or future
/// MIR optimizations can theoretically optimize out segments of a MIR, it may not be possible to
/// guarantee this, so the second pass ensures the `CoverageInfo` counts include all referenced IDs.
struct CoverageVisitor {
info: CoverageInfo,
add_missing_operands: bool,
}
impl CoverageVisitor {
// If an expression operand is encountered with an ID outside the range of known counters and
// expressions, the only way to determine if the ID is a counter ID or an expression ID is to
// assume a maximum possible counter ID value.
const MAX_COUNTER_GUARD: u32 = (u32::MAX / 2) + 1;
#[inline(always)]
fn update_num_counters(&mut self, counter_id: u32) {
self.info.num_counters = std::cmp::max(self.info.num_counters, counter_id + 1);
@ -62,7 +59,10 @@ fn update_from_expression_operand(&mut self, operand_id: u32) {
if operand_id >= self.info.num_counters {
let operand_as_expression_index = u32::MAX - operand_id;
if operand_as_expression_index >= self.info.num_expressions {
if operand_id <= Self::MAX_COUNTER_GUARD {
if operand_id <= counters::MAX_COUNTER_GUARD {
// Since the complete range of counter and expression IDs is not known here, the
// only way to determine if the ID is a counter ID or an expression ID is to
// assume a maximum possible counter ID value.
self.update_num_counters(operand_id)
} else {
self.update_num_expressions(operand_id)

6
compiler/rustc_mir/src/transform/coverage/spans.rs
View File

@ -388,8 +388,7 @@ fn bcb_to_initial_coverage_spans(
bcb_data
.basic_blocks
.iter()
.map(|bbref| {
let bb = *bbref;
.flat_map(|&bb| {
let data = &self.mir_body[bb];
data.statements
.iter()
@ -404,7 +403,6 @@ fn bcb_to_initial_coverage_spans(
.map(|span| CoverageSpan::for_terminator(span, bcb, bb)),
)
})
.flatten()
.collect()
}
@ -733,7 +731,7 @@ fn filtered_terminator_span(terminator: &'a Terminator<'tcx>, body_span: Span) -
// However, in other cases, a visible `CoverageSpan` is not wanted, but the `Goto`
// block must still be counted (for example, to contribute its count to an `Expression`
// that reports the execution count for some other block). In these cases, the code region
// is set to `None`.
// is set to `None`. (See `Instrumentor::is_code_region_redundant()`.)
TerminatorKind::Goto { .. } => {
Some(function_source_span(terminator.source_info.span.shrink_to_hi(), body_span))
}