Skip to content

Commit

Permalink
Rollup merge of #78300 - richkadel:coverage-cx, r=wesleywiser
Browse files Browse the repository at this point in the history
Make codegen coverage_context optional, and check

Addresses Issue #78286

Libraries compiled with coverage and linked with out enabling coverage
would fail when attempting to add the library's coverage statements to
the codegen coverage context (None).

Now, if coverage statements are encountered while compiling / linking
with `-Z instrument-coverage` disabled, codegen will *not* attempt to
add code regions to a coverage map, and it will not inject the LLVM
instrprof_increment intrinsic calls.
  • Loading branch information
jonas-schievink authored Oct 24, 2020
2 parents a07fc3d + f75a236 commit da48646
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 50 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_llvm/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,8 +324,8 @@ impl<'ll, 'tcx> CodegenCx<'ll, 'tcx> {
}

#[inline]
pub fn coverage_context(&'a self) -> &'a coverageinfo::CrateCoverageContext<'tcx> {
self.coverage_cx.as_ref().unwrap()
pub fn coverage_context(&'a self) -> Option<&'a coverageinfo::CrateCoverageContext<'tcx>> {
self.coverage_cx.as_ref()
}
}

Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_codegen_llvm/src/coverageinfo/mapgen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ use tracing::debug;
/// undocumented details in Clang's implementation (that may or may not be important) were also
/// replicated for Rust's Coverage Map.
pub fn finalize<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>) {
let function_coverage_map = cx.coverage_context().take_function_coverage_map();
let function_coverage_map = match cx.coverage_context() {
Some(ctx) => ctx.take_function_coverage_map(),
None => return,
};
if function_coverage_map.is_empty() {
// This module has no functions with coverage instrumentation
return;
Expand Down
79 changes: 47 additions & 32 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,22 @@ impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
function_source_hash: u64,
id: CounterValueReference,
region: CodeRegion,
) {
debug!(
"adding counter to coverage_regions: instance={:?}, function_source_hash={}, id={:?}, \
at {:?}",
instance, function_source_hash, id, region,
);
let mut coverage_regions = self.coverage_context().function_coverage_map.borrow_mut();
coverage_regions
.entry(instance)
.or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
.add_counter(function_source_hash, id, region);
) -> bool {
if let Some(coverage_context) = self.coverage_context() {
debug!(
"adding counter to coverage_regions: instance={:?}, function_source_hash={}, id={:?}, \
at {:?}",
instance, function_source_hash, id, region,
);
let mut coverage_regions = coverage_context.function_coverage_map.borrow_mut();
coverage_regions
.entry(instance)
.or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
.add_counter(function_source_hash, id, region);
true
} else {
false
}
}

fn add_counter_expression_region(
Expand All @@ -85,29 +90,39 @@ impl CoverageInfoBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
op: Op,
rhs: ExpressionOperandId,
region: CodeRegion,
) {
debug!(
"adding counter expression to coverage_regions: instance={:?}, id={:?}, {:?} {:?} {:?}, \
at {:?}",
instance, id, lhs, op, rhs, region,
);
let mut coverage_regions = self.coverage_context().function_coverage_map.borrow_mut();
coverage_regions
.entry(instance)
.or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
.add_counter_expression(id, lhs, op, rhs, region);
) -> bool {
if let Some(coverage_context) = self.coverage_context() {
debug!(
"adding counter expression to coverage_regions: instance={:?}, id={:?}, {:?} {:?} {:?}, \
at {:?}",
instance, id, lhs, op, rhs, region,
);
let mut coverage_regions = coverage_context.function_coverage_map.borrow_mut();
coverage_regions
.entry(instance)
.or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
.add_counter_expression(id, lhs, op, rhs, region);
true
} else {
false
}
}

fn add_unreachable_region(&mut self, instance: Instance<'tcx>, region: CodeRegion) {
debug!(
"adding unreachable code to coverage_regions: instance={:?}, at {:?}",
instance, region,
);
let mut coverage_regions = self.coverage_context().function_coverage_map.borrow_mut();
coverage_regions
.entry(instance)
.or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
.add_unreachable_region(region);
fn add_unreachable_region(&mut self, instance: Instance<'tcx>, region: CodeRegion) -> bool {
if let Some(coverage_context) = self.coverage_context() {
debug!(
"adding unreachable code to coverage_regions: instance={:?}, at {:?}",
instance, region,
);
let mut coverage_regions = coverage_context.function_coverage_map.borrow_mut();
coverage_regions
.entry(instance)
.or_insert_with(|| FunctionCoverage::new(self.tcx, instance))
.add_unreachable_region(region);
true
} else {
false
}
}
}

Expand Down
24 changes: 12 additions & 12 deletions compiler/rustc_codegen_ssa/src/mir/coverageinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,19 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
let Coverage { kind, code_region } = coverage;
match kind {
CoverageKind::Counter { function_source_hash, id } => {
bx.add_counter_region(self.instance, function_source_hash, id, code_region);
if bx.add_counter_region(self.instance, function_source_hash, id, code_region) {
let coverageinfo = bx.tcx().coverageinfo(self.instance.def_id());

let coverageinfo = bx.tcx().coverageinfo(self.instance.def_id());

let fn_name = bx.create_pgo_func_name_var(self.instance);
let hash = bx.const_u64(function_source_hash);
let num_counters = bx.const_u32(coverageinfo.num_counters);
let id = bx.const_u32(u32::from(id));
debug!(
"codegen intrinsic instrprof.increment(fn_name={:?}, hash={:?}, num_counters={:?}, index={:?})",
fn_name, hash, num_counters, id,
);
bx.instrprof_increment(fn_name, hash, num_counters, id);
let fn_name = bx.create_pgo_func_name_var(self.instance);
let hash = bx.const_u64(function_source_hash);
let num_counters = bx.const_u32(coverageinfo.num_counters);
let id = bx.const_u32(u32::from(id));
debug!(
"codegen intrinsic instrprof.increment(fn_name={:?}, hash={:?}, num_counters={:?}, index={:?})",
fn_name, hash, num_counters, id,
);
bx.instrprof_increment(fn_name, hash, num_counters, id);
}
}
CoverageKind::Expression { id, lhs, op, rhs } => {
bx.add_counter_expression_region(self.instance, id, lhs, op, rhs, code_region);
Expand Down
12 changes: 9 additions & 3 deletions compiler/rustc_codegen_ssa/src/traits/coverageinfo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,18 @@ 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 counter was added to the coverage map; false if `-Z instrument-coverage`
/// is not enabled (a coverage map is not being generated).
fn add_counter_region(
&mut self,
instance: Instance<'tcx>,
function_source_hash: u64,
id: CounterValueReference,
region: CodeRegion,
);
) -> bool;

/// Returns true if the expression was added to the coverage map; false if
/// `-Z instrument-coverage` is not enabled (a coverage map is not being generated).
fn add_counter_expression_region(
&mut self,
instance: Instance<'tcx>,
Expand All @@ -25,7 +29,9 @@ pub trait CoverageInfoBuilderMethods<'tcx>: BackendTypes {
op: Op,
rhs: ExpressionOperandId,
region: CodeRegion,
);
) -> bool;

fn add_unreachable_region(&mut self, instance: Instance<'tcx>, region: CodeRegion);
/// Returns true if the region was added to the coverage map; false if `-Z instrument-coverage`
/// is not enabled (a coverage map is not being generated).
fn add_unreachable_region(&mut self, instance: Instance<'tcx>, region: CodeRegion) -> bool;
}

0 comments on commit da48646

Please sign in to comment.