Skip to content

Commit

Permalink
Auto merge of #77080 - richkadel:llvm-coverage-counters-2, r=tmandry
Browse files Browse the repository at this point in the history
Working branch-level code coverage

Add a generalized implementation for computing branch-level coverage spans.

This iteration resolves some of the challenges I had identified a few weeks ago.

I've tried to implement a solution that is general enough to work for a lot of different graphs/patterns. It's encouraging to see the results on fairly large and complex crates seem to meet my expectations. This may be a "functionally complete" implementation.

Except for bug fixes or edge cases I haven't run into yet, the next and essentially final step, I think, is to replace some Counters with CounterExpressions (where their counter values can be computed by adding or subtracting other counters/expressions).

Examples of branch-level coverage support enabled in this PR:

* https://github.com/richkadel/rust/blob/llvm-coverage-counters-2/src/test/run-make-fulldeps/instrument-coverage-cov-reports-base/expected_show_coverage.coverage_of_drop_trait.txt
* https://github.com/richkadel/rust/blob/llvm-coverage-counters-2/src/test/run-make-fulldeps/instrument-coverage-cov-reports-base/expected_show_coverage.coverage_of_if.txt
* https://github.com/richkadel/rust/blob/llvm-coverage-counters-2/src/test/run-make-fulldeps/instrument-coverage-cov-reports-base/expected_show_coverage.coverage_of_if_else.txt
* https://github.com/richkadel/rust/blob/llvm-coverage-counters-2/src/test/run-make-fulldeps/instrument-coverage-cov-reports-base/expected_show_coverage.coverage_of_simple_loop.txt
* https://github.com/richkadel/rust/blob/llvm-coverage-counters-2/src/test/run-make-fulldeps/instrument-coverage-cov-reports-base/expected_show_coverage.coverage_of_simple_match.txt
* ... _and others in the same directory_

Examples of coverage analysis results (MIR spanview files) used to inject counters in the right `BasicBlocks`:

* https://htmlpreview.github.io/?https://github.com/richkadel/rust/blob/llvm-coverage-counters-2/src/test/run-make-fulldeps/instrument-coverage-mir-cov-html-base/expected_mir_dump.coverage_of_drop_trait/coverage_of_drop_trait.main.-------.InstrumentCoverage.0.html
* https://htmlpreview.github.io/?https://github.com/richkadel/rust/blob/llvm-coverage-counters-2/src/test/run-make-fulldeps/instrument-coverage-mir-cov-html-base/expected_mir_dump.coverage_of_if/coverage_of_if.main.-------.InstrumentCoverage.0.html
* https://htmlpreview.github.io/?https://github.com/richkadel/rust/blob/llvm-coverage-counters-2/src/test/run-make-fulldeps/instrument-coverage-mir-cov-html-base/expected_mir_dump.coverage_of_if_else/coverage_of_if_else.main.-------.InstrumentCoverage.0.html
* https://htmlpreview.github.io/?https://github.com/richkadel/rust/blob/llvm-coverage-counters-2/src/test/run-make-fulldeps/instrument-coverage-mir-cov-html-base/expected_mir_dump.coverage_of_simple_loop/coverage_of_simple_loop.main.-------.InstrumentCoverage.0.html
* https://htmlpreview.github.io/?https://github.com/richkadel/rust/blob/llvm-coverage-counters-2/src/test/run-make-fulldeps/instrument-coverage-mir-cov-html-base/expected_mir_dump.coverage_of_simple_match/coverage_of_simple_match.main.-------.InstrumentCoverage.0.html
* ... _and others in the same directory_

Here is some sample coverage output after compiling a few real-world crates with the new branch-level coverage features:

<img width="801" alt="Screen Shot 2020-09-25 at 1 03 11 PM" src="https://user-images.githubusercontent.com/3827298/94316848-fd882c00-ff39-11ea-9cff-0402d3abd1e7.png">
<img width="721" alt="Screen Shot 2020-09-25 at 1 00 36 PM" src="https://user-images.githubusercontent.com/3827298/94316886-11cc2900-ff3a-11ea-9d03-80b26c8a5173.png">
<img width="889" alt="Screen Shot 2020-09-25 at 12 54 57 PM" src="https://user-images.githubusercontent.com/3827298/94316900-18f33700-ff3a-11ea-8a80-58f67d84b8de.png">

r? `@tmandry`
FYI: `@wesleywiser`
  • Loading branch information
bors committed Oct 5, 2020
2 parents ea7e131 + 6f62766 commit a1dfd24
Show file tree
Hide file tree
Showing 151 changed files with 19,001 additions and 1,878 deletions.
3 changes: 2 additions & 1 deletion Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2938,8 +2938,9 @@ dependencies = [

[[package]]
name = "rust-demangler"
version = "0.0.0"
version = "0.0.1"
dependencies = [
"regex",
"rustc-demangle",
]

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ impl CoverageMapGenerator {
let (filenames_index, _) = self.filenames.insert_full(c_filename);
virtual_file_mapping.push(filenames_index as u32);
}
debug!("Adding counter {:?} to map for {:?}", counter, region,);
mapping_regions.push(CounterMappingRegion::code_region(
counter,
current_file_id,
Expand Down
13 changes: 10 additions & 3 deletions compiler/rustc_codegen_ssa/src/coverageinfo/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ impl FunctionCoverage {
let id_to_counter =
|new_indexes: &IndexVec<InjectedExpressionIndex, MappedExpressionIndex>,
id: ExpressionOperandId| {
if id.index() < self.counters.len() {
if id == ExpressionOperandId::ZERO {
Some(Counter::zero())
} else if id.index() < self.counters.len() {
let index = CounterValueReference::from(id.index());
self.counters
.get(index)
Expand Down Expand Up @@ -179,14 +181,19 @@ impl FunctionCoverage {
// been assigned a `new_index`.
let mapped_expression_index =
MappedExpressionIndex::from(counter_expressions.len());
counter_expressions.push(CounterExpression::new(
let expression = CounterExpression::new(
lhs_counter,
match op {
Op::Add => ExprKind::Add,
Op::Subtract => ExprKind::Subtract,
},
rhs_counter,
));
);
debug!(
"Adding expression {:?} = {:?} at {:?}",
mapped_expression_index, expression, region
);
counter_expressions.push(expression);
new_indexes[original_index] = mapped_expression_index;
expression_regions.push((Counter::expression(mapped_expression_index), region));
}
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_data_structures/src/graph/dominators/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use super::iterate::reverse_post_order;
use super::ControlFlowGraph;
use rustc_index::vec::{Idx, IndexVec};
use std::borrow::BorrowMut;
use std::cmp::Ordering;

#[cfg(test)]
mod tests;
Expand Down Expand Up @@ -108,6 +109,14 @@ impl<Node: Idx> Dominators<Node> {
// FIXME -- could be optimized by using post-order-rank
self.dominators(node).any(|n| n == dom)
}

/// Provide deterministic ordering of nodes such that, if any two nodes have a dominator
/// relationship, the dominator will always precede the dominated. (The relative ordering
/// of two unrelated nodes will also be consistent, but otherwise the order has no
/// meaning.) This method cannot be used to determine if either Node dominates the other.
pub fn rank_partial_cmp(&self, lhs: Node, rhs: Node) -> Option<Ordering> {
self.post_order_rank[lhs].partial_cmp(&self.post_order_rank[rhs])
}
}

pub struct Iter<'dom, Node: Idx> {
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_middle/src/mir/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,20 @@ rustc_index::newtype_index! {
}
}

impl ExpressionOperandId {
/// An expression operand for a "zero counter", as described in the following references:
///
/// * https://github.com/rust-lang/llvm-project/blob/llvmorg-8.0.0/llvm/docs/CoverageMappingFormat.rst#counter
/// * https://github.com/rust-lang/llvm-project/blob/llvmorg-8.0.0/llvm/docs/CoverageMappingFormat.rst#tag
/// * https://github.com/rust-lang/llvm-project/blob/llvmorg-8.0.0/llvm/docs/CoverageMappingFormat.rst#counter-expressions
///
/// This operand can be used to count two or more separate code regions with a single counter,
/// if they run sequentially with no branches, by injecting the `Counter` in a `BasicBlock` for
/// one of the code regions, and inserting `CounterExpression`s ("add ZERO to the counter") in
/// the coverage map for the other code regions.
pub const ZERO: Self = Self::from_u32(0);
}

rustc_index::newtype_index! {
pub struct CounterValueReference {
derive [HashStable]
Expand All @@ -22,6 +36,11 @@ rustc_index::newtype_index! {
}
}

impl CounterValueReference {
// Counters start at 1 to reserve 0 for ExpressionOperandId::ZERO.
pub const START: Self = Self::from_u32(1);
}

rustc_index::newtype_index! {
pub struct InjectedExpressionIndex {
derive [HashStable]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ macro_rules! make_mir_visitor {
}

fn super_coverage(&mut self,
_kind: & $($mutability)? Coverage,
_coverage: & $($mutability)? Coverage,
_location: Location) {
}

Expand Down
Loading

0 comments on commit a1dfd24

Please sign in to comment.