Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MCDC coverage: support nested decision coverage #124255

Merged
merged 4 commits into from
Apr 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 20 additions & 11 deletions compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use rustc_target::abi::{self, call::FnAbi, Align, Size, WrappingRange};
use rustc_target::spec::{HasTargetSpec, SanitizerSet, Target};
use smallvec::SmallVec;
use std::borrow::Cow;
use std::ffi::CString;
use std::iter;
use std::ops::Deref;
use std::ptr;
Expand Down Expand Up @@ -1708,7 +1709,8 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
fn_name: &'ll Value,
hash: &'ll Value,
bitmap_bytes: &'ll Value,
) -> &'ll Value {
max_decision_depth: u32,
) -> Vec<&'ll Value> {
debug!("mcdc_parameters() with args ({:?}, {:?}, {:?})", fn_name, hash, bitmap_bytes);

assert!(llvm_util::get_version() >= (18, 0, 0), "MCDC intrinsics require LLVM 18 or later");
Expand All @@ -1721,6 +1723,8 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
let args = &[fn_name, hash, bitmap_bytes];
let args = self.check_call("call", llty, llfn, args);

let mut cond_bitmaps = vec![];

unsafe {
let _ = llvm::LLVMRustBuildCall(
self.llbuilder,
Expand All @@ -1732,17 +1736,22 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
0 as c_uint,
);
// Create condition bitmap named `mcdc.addr`.
let mut bx = Builder::with_cx(self.cx);
bx.position_at_start(llvm::LLVMGetFirstBasicBlock(self.llfn()));
let cond_bitmap = {
let alloca =
llvm::LLVMBuildAlloca(bx.llbuilder, bx.cx.type_i32(), c"mcdc.addr".as_ptr());
llvm::LLVMSetAlignment(alloca, 4);
alloca
};
bx.store(self.const_i32(0), cond_bitmap, self.tcx().data_layout.i32_align.abi);
cond_bitmap
for i in 0..=max_decision_depth {
let mut bx = Builder::with_cx(self.cx);
bx.position_at_start(llvm::LLVMGetFirstBasicBlock(self.llfn()));

let name = CString::new(format!("mcdc.addr.{i}")).unwrap();
let cond_bitmap = {
let alloca =
llvm::LLVMBuildAlloca(bx.llbuilder, bx.cx.type_i32(), name.as_ptr());
llvm::LLVMSetAlignment(alloca, 4);
alloca
};
bx.store(self.const_i32(0), cond_bitmap, self.tcx().data_layout.i32_align.abi);
cond_bitmaps.push(cond_bitmap);
}
}
cond_bitmaps
}

pub(crate) fn mcdc_tvbitmap_update(
Expand Down
25 changes: 18 additions & 7 deletions compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub struct CrateCoverageContext<'ll, 'tcx> {
pub(crate) function_coverage_map:
RefCell<FxIndexMap<Instance<'tcx>, FunctionCoverageCollector<'tcx>>>,
pub(crate) pgo_func_name_var_map: RefCell<FxHashMap<Instance<'tcx>, &'ll llvm::Value>>,
pub(crate) mcdc_condition_bitmap_map: RefCell<FxHashMap<Instance<'tcx>, &'ll llvm::Value>>,
pub(crate) mcdc_condition_bitmap_map: RefCell<FxHashMap<Instance<'tcx>, Vec<&'ll llvm::Value>>>,
}

impl<'ll, 'tcx> CrateCoverageContext<'ll, 'tcx> {
Expand All @@ -49,9 +49,20 @@ impl<'ll, 'tcx> CrateCoverageContext<'ll, 'tcx> {
}

/// LLVM use a temp value to record evaluated mcdc test vector of each decision, which is called condition bitmap.
/// This value is named `mcdc.addr` (same as clang) and is a 32-bit integer.
fn try_get_mcdc_condition_bitmap(&self, instance: &Instance<'tcx>) -> Option<&'ll llvm::Value> {
self.mcdc_condition_bitmap_map.borrow().get(instance).copied()
/// In order to handle nested decisions, several condition bitmaps can be
/// allocated for a function body.
/// These values are named `mcdc.addr.{i}` and are a 32-bit integers.
/// They respectively hold the condition bitmaps for decisions with a depth of `i`.
fn try_get_mcdc_condition_bitmap(
&self,
instance: &Instance<'tcx>,
decision_depth: u16,
) -> Option<&'ll llvm::Value> {
self.mcdc_condition_bitmap_map
.borrow()
.get(instance)
.and_then(|bitmap_map| bitmap_map.get(decision_depth as usize))
.copied() // Dereference Option<&&Value> to Option<&Value>
}
}

Expand Down Expand Up @@ -151,7 +162,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
"ConditionId of evaluated conditions should never be zero"
);
let cond_bitmap = coverage_context
.try_get_mcdc_condition_bitmap(&instance)
.try_get_mcdc_condition_bitmap(&instance, 0)
.expect("mcdc cond bitmap should have been allocated for updating");
let cond_loc = bx.const_i32(id.as_u32() as i32 - 1);
let bool_value = bx.const_bool(value);
Expand All @@ -162,7 +173,7 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
CoverageKind::TestVectorBitmapUpdate { bitmap_idx } => {
drop(coverage_map);
let cond_bitmap = coverage_context
.try_get_mcdc_condition_bitmap(&instance)
.try_get_mcdc_condition_bitmap(&instance, 0)
.expect("mcdc cond bitmap should have been allocated for merging into the global bitmap");
let bitmap_bytes = bx.tcx().coverage_ids_info(instance.def).mcdc_bitmap_bytes;
assert!(bitmap_idx < bitmap_bytes, "bitmap index of the decision out of range");
Expand Down Expand Up @@ -195,7 +206,7 @@ fn ensure_mcdc_parameters<'ll, 'tcx>(
let fn_name = bx.get_pgo_func_name_var(instance);
let hash = bx.const_u64(function_coverage_info.function_source_hash);
let bitmap_bytes = bx.const_u32(function_coverage_info.mcdc_bitmap_bytes);
let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes);
let cond_bitmap = bx.mcdc_parameters(fn_name, hash, bitmap_bytes, 1_u32);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should actually be 0, not 1. But since this is temporary code anyway, and the only effect is an extra unused bitmap, it's OK to not worry about it.

bx.coverage_context()
.expect("already checked above")
.mcdc_condition_bitmap_map
Expand Down