Skip to content

Commit 83e745f

Browse files
author
zhuyunxing
committed
coverage. Adapt to llvm optimization for conditions limits
1 parent c1bb8fd commit 83e745f

File tree

13 files changed

+266
-189
lines changed

13 files changed

+266
-189
lines changed

compiler/rustc_codegen_llvm/src/builder.rs

+14-42
Original file line numberDiff line numberDiff line change
@@ -1719,9 +1719,9 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
17191719
&mut self,
17201720
fn_name: &'ll Value,
17211721
hash: &'ll Value,
1722-
bitmap_bytes: &'ll Value,
1722+
bitmap_bits: &'ll Value,
17231723
) {
1724-
debug!("mcdc_parameters() with args ({:?}, {:?}, {:?})", fn_name, hash, bitmap_bytes);
1724+
debug!("mcdc_parameters() with args ({:?}, {:?}, {:?})", fn_name, hash, bitmap_bits);
17251725

17261726
assert!(llvm_util::get_version() >= (19, 0, 0), "MCDC intrinsics require LLVM 19 or later");
17271727

@@ -1730,7 +1730,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
17301730
&[self.cx.type_ptr(), self.cx.type_i64(), self.cx.type_i32()],
17311731
self.cx.type_void(),
17321732
);
1733-
let args = &[fn_name, hash, bitmap_bytes];
1733+
let args = &[fn_name, hash, bitmap_bits];
17341734
let args = self.check_call("call", llty, llfn, args);
17351735

17361736
unsafe {
@@ -1750,29 +1750,22 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
17501750
&mut self,
17511751
fn_name: &'ll Value,
17521752
hash: &'ll Value,
1753-
bitmap_bytes: &'ll Value,
17541753
bitmap_index: &'ll Value,
17551754
mcdc_temp: &'ll Value,
17561755
) {
17571756
debug!(
1758-
"mcdc_tvbitmap_update() with args ({:?}, {:?}, {:?}, {:?}, {:?})",
1759-
fn_name, hash, bitmap_bytes, bitmap_index, mcdc_temp
1757+
"mcdc_tvbitmap_update() with args ({:?}, {:?}, {:?}, {:?})",
1758+
fn_name, hash, bitmap_index, mcdc_temp
17601759
);
17611760
assert!(llvm_util::get_version() >= (19, 0, 0), "MCDC intrinsics require LLVM 19 or later");
17621761

17631762
let llfn =
17641763
unsafe { llvm::LLVMRustGetInstrProfMCDCTVBitmapUpdateIntrinsic(self.cx().llmod) };
17651764
let llty = self.cx.type_func(
1766-
&[
1767-
self.cx.type_ptr(),
1768-
self.cx.type_i64(),
1769-
self.cx.type_i32(),
1770-
self.cx.type_i32(),
1771-
self.cx.type_ptr(),
1772-
],
1765+
&[self.cx.type_ptr(), self.cx.type_i64(), self.cx.type_i32(), self.cx.type_ptr()],
17731766
self.cx.type_void(),
17741767
);
1775-
let args = &[fn_name, hash, bitmap_bytes, bitmap_index, mcdc_temp];
1768+
let args = &[fn_name, hash, bitmap_index, mcdc_temp];
17761769
let args = self.check_call("call", llty, llfn, args);
17771770
unsafe {
17781771
let _ = llvm::LLVMRustBuildCall(
@@ -1792,38 +1785,17 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
17921785
&mut self,
17931786
fn_name: &'ll Value,
17941787
hash: &'ll Value,
1795-
cond_loc: &'ll Value,
1788+
cond_index: &'ll Value,
17961789
mcdc_temp: &'ll Value,
1797-
bool_value: &'ll Value,
17981790
) {
17991791
debug!(
1800-
"mcdc_condbitmap_update() with args ({:?}, {:?}, {:?}, {:?}, {:?})",
1801-
fn_name, hash, cond_loc, mcdc_temp, bool_value
1792+
"mcdc_condbitmap_update() with args ({:?}, {:?}, {:?}, {:?})",
1793+
fn_name, hash, cond_index, mcdc_temp
18021794
);
18031795
assert!(llvm_util::get_version() >= (19, 0, 0), "MCDC intrinsics require LLVM 19 or later");
1804-
let llfn = unsafe { llvm::LLVMRustGetInstrProfMCDCCondBitmapIntrinsic(self.cx().llmod) };
1805-
let llty = self.cx.type_func(
1806-
&[
1807-
self.cx.type_ptr(),
1808-
self.cx.type_i64(),
1809-
self.cx.type_i32(),
1810-
self.cx.type_ptr(),
1811-
self.cx.type_i1(),
1812-
],
1813-
self.cx.type_void(),
1814-
);
1815-
let args = &[fn_name, hash, cond_loc, mcdc_temp, bool_value];
1816-
self.check_call("call", llty, llfn, args);
1817-
unsafe {
1818-
let _ = llvm::LLVMRustBuildCall(
1819-
self.llbuilder,
1820-
llty,
1821-
llfn,
1822-
args.as_ptr() as *const &llvm::Value,
1823-
args.len() as c_uint,
1824-
[].as_ptr(),
1825-
0 as c_uint,
1826-
);
1827-
}
1796+
let align = self.tcx.data_layout.i32_align.abi;
1797+
let current_tv_index = self.load(self.cx.type_i32(), mcdc_temp, align);
1798+
let new_tv_index = self.add(current_tv_index, cond_index);
1799+
self.store(new_tv_index, mcdc_temp, align);
18281800
}
18291801
}

compiler/rustc_codegen_llvm/src/coverageinfo/mod.rs

+11-16
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,14 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
100100
};
101101

102102
// If there are no MC/DC bitmaps to set up, return immediately.
103-
if function_coverage_info.mcdc_bitmap_bytes == 0 {
103+
if function_coverage_info.mcdc_bitmap_bits == 0 {
104104
return;
105105
}
106106

107107
let fn_name = self.get_pgo_func_name_var(instance);
108108
let hash = self.const_u64(function_coverage_info.function_source_hash);
109-
let bitmap_bytes = self.const_u32(function_coverage_info.mcdc_bitmap_bytes);
110-
self.mcdc_parameters(fn_name, hash, bitmap_bytes);
109+
let bitmap_bits = self.const_u32(function_coverage_info.mcdc_bitmap_bits as u32);
110+
self.mcdc_parameters(fn_name, hash, bitmap_bits);
111111

112112
// Create pointers named `mcdc.addr.{i}` to stack-allocated condition bitmaps.
113113
let mut cond_bitmaps = vec![];
@@ -187,35 +187,30 @@ impl<'tcx> CoverageInfoBuilderMethods<'tcx> for Builder<'_, '_, 'tcx> {
187187
CoverageKind::ExpressionUsed { id } => {
188188
func_coverage.mark_expression_id_seen(id);
189189
}
190-
CoverageKind::CondBitmapUpdate { id, value, decision_depth } => {
190+
CoverageKind::CondBitmapUpdate { index, decision_depth } => {
191191
drop(coverage_map);
192-
assert_ne!(
193-
id.as_u32(),
194-
0,
195-
"ConditionId of evaluated conditions should never be zero"
196-
);
197192
let cond_bitmap = coverage_context
198193
.try_get_mcdc_condition_bitmap(&instance, decision_depth)
199194
.expect("mcdc cond bitmap should have been allocated for updating");
200-
let cond_loc = bx.const_i32(id.as_u32() as i32 - 1);
201-
let bool_value = bx.const_bool(value);
195+
let cond_index = bx.const_i32(index as i32);
202196
let fn_name = bx.get_pgo_func_name_var(instance);
203197
let hash = bx.const_u64(function_coverage_info.function_source_hash);
204-
bx.mcdc_condbitmap_update(fn_name, hash, cond_loc, cond_bitmap, bool_value);
198+
bx.mcdc_condbitmap_update(fn_name, hash, cond_index, cond_bitmap);
205199
}
206200
CoverageKind::TestVectorBitmapUpdate { bitmap_idx, decision_depth } => {
207201
drop(coverage_map);
208202
let cond_bitmap = coverage_context
209203
.try_get_mcdc_condition_bitmap(&instance, decision_depth)
210204
.expect("mcdc cond bitmap should have been allocated for merging into the global bitmap");
211-
let bitmap_bytes = function_coverage_info.mcdc_bitmap_bytes;
212-
assert!(bitmap_idx < bitmap_bytes, "bitmap index of the decision out of range");
205+
assert!(
206+
bitmap_idx as usize <= function_coverage_info.mcdc_bitmap_bits,
207+
"bitmap index of the decision out of range"
208+
);
213209

214210
let fn_name = bx.get_pgo_func_name_var(instance);
215211
let hash = bx.const_u64(function_coverage_info.function_source_hash);
216-
let bitmap_bytes = bx.const_u32(bitmap_bytes);
217212
let bitmap_index = bx.const_u32(bitmap_idx);
218-
bx.mcdc_tvbitmap_update(fn_name, hash, bitmap_bytes, bitmap_index, cond_bitmap);
213+
bx.mcdc_tvbitmap_update(fn_name, hash, bitmap_index, cond_bitmap);
219214
}
220215
}
221216
}

compiler/rustc_codegen_llvm/src/llvm/ffi.rs

-1
Original file line numberDiff line numberDiff line change
@@ -1635,7 +1635,6 @@ extern "C" {
16351635
pub fn LLVMRustGetInstrProfIncrementIntrinsic(M: &Module) -> &Value;
16361636
pub fn LLVMRustGetInstrProfMCDCParametersIntrinsic(M: &Module) -> &Value;
16371637
pub fn LLVMRustGetInstrProfMCDCTVBitmapUpdateIntrinsic(M: &Module) -> &Value;
1638-
pub fn LLVMRustGetInstrProfMCDCCondBitmapIntrinsic(M: &Module) -> &Value;
16391638

16401639
pub fn LLVMRustBuildCall<'a>(
16411640
B: &Builder<'a>,

compiler/rustc_middle/src/mir/coverage.rs

+14-11
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,8 @@ pub enum CoverageKind {
136136

137137
/// Marks the point in MIR control flow represented by a evaluated condition.
138138
///
139-
/// This is eventually lowered to `llvm.instrprof.mcdc.condbitmap.update` in LLVM IR.
140-
CondBitmapUpdate { id: ConditionId, value: bool, decision_depth: u16 },
139+
/// This is eventually lowered to instruments updating mcdc temp variables.
140+
CondBitmapUpdate { index: u32, decision_depth: u16 },
141141

142142
/// Marks the point in MIR control flow represented by a evaluated decision.
143143
///
@@ -153,14 +153,8 @@ impl Debug for CoverageKind {
153153
BlockMarker { id } => write!(fmt, "BlockMarker({:?})", id.index()),
154154
CounterIncrement { id } => write!(fmt, "CounterIncrement({:?})", id.index()),
155155
ExpressionUsed { id } => write!(fmt, "ExpressionUsed({:?})", id.index()),
156-
CondBitmapUpdate { id, value, decision_depth } => {
157-
write!(
158-
fmt,
159-
"CondBitmapUpdate({:?}, {:?}, depth={:?})",
160-
id.index(),
161-
value,
162-
decision_depth
163-
)
156+
CondBitmapUpdate { index, decision_depth } => {
157+
write!(fmt, "CondBitmapUpdate(index={:?}, depth={:?})", index, decision_depth)
164158
}
165159
TestVectorBitmapUpdate { bitmap_idx, decision_depth } => {
166160
write!(fmt, "TestVectorUpdate({:?}, depth={:?})", bitmap_idx, decision_depth)
@@ -274,7 +268,7 @@ pub struct Mapping {
274268
pub struct FunctionCoverageInfo {
275269
pub function_source_hash: u64,
276270
pub num_counters: usize,
277-
pub mcdc_bitmap_bytes: u32,
271+
pub mcdc_bitmap_bits: usize,
278272
pub expressions: IndexVec<ExpressionId, Expression>,
279273
pub mappings: Vec<Mapping>,
280274
/// The depth of the deepest decision is used to know how many
@@ -317,6 +311,14 @@ pub struct MCDCBranchSpan {
317311
pub span: Span,
318312
pub condition_info: ConditionInfo,
319313
pub markers: MCDCBranchMarkers,
314+
pub false_index: usize,
315+
pub true_index: usize,
316+
}
317+
318+
impl MCDCBranchSpan {
319+
pub fn new(span: Span, condition_info: ConditionInfo, markers: MCDCBranchMarkers) -> Self {
320+
Self { span, condition_info, markers, false_index: usize::MAX, true_index: usize::MAX }
321+
}
320322
}
321323

322324
#[derive(Clone, Debug)]
@@ -340,4 +342,5 @@ pub struct MCDCDecisionSpan {
340342
pub span: Span,
341343
pub end_markers: Vec<BlockMarkerId>,
342344
pub decision_depth: u16,
345+
pub num_test_vectors: usize,
343346
}

compiler/rustc_middle/src/mir/pretty.rs

+9-5
Original file line numberDiff line numberDiff line change
@@ -503,18 +503,22 @@ fn write_coverage_branch_info(
503503
)?;
504504
}
505505

506-
for (coverage::MCDCDecisionSpan { span, end_markers, decision_depth }, conditions) in mcdc_spans
506+
for (
507+
coverage::MCDCDecisionSpan { span, end_markers, decision_depth, num_test_vectors },
508+
conditions,
509+
) in mcdc_spans
507510
{
508511
let num_conditions = conditions.len();
509512
writeln!(
510513
w,
511-
"{INDENT}coverage mcdc decision {{ num_conditions: {num_conditions:?}, end: {end_markers:?}, depth: {decision_depth:?} }} => {span:?}"
514+
"{INDENT}coverage mcdc decision {{ num_conditions: {num_conditions:?}, end: {end_markers:?}, depth: {decision_depth:?}, num_test_vectors: {num_test_vectors} }} => {span:?}"
512515
)?;
513-
for coverage::MCDCBranchSpan { span, condition_info, markers } in conditions {
516+
for coverage::MCDCBranchSpan { span, condition_info, markers, false_index, true_index } in
517+
conditions
518+
{
514519
writeln!(
515520
w,
516-
"{INDENT}coverage mcdc branch {{ condition_id: {:?}, markers: {markers:?} }} => {span:?}",
517-
condition_info.condition_id
521+
"{INDENT}coverage mcdc branch {{ condition_info: {condition_info:?}, markers: {markers:?}, indices: {{ false: {false_index}, true: {true_index} }} }} => {span:?}",
518522
)?;
519523
}
520524
}

compiler/rustc_mir_build/src/build/coverageinfo/mcdc.rs

+86-8
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::collections::VecDeque;
22

33
use rustc_data_structures::fx::FxIndexMap;
4+
use rustc_index::IndexVec;
45
use rustc_middle::bug;
56
use rustc_middle::mir::coverage::{
67
BlockMarkerId, ConditionId, ConditionInfo, DecisionId, MCDCBranchMarkers, MCDCBranchSpan,
@@ -14,10 +15,9 @@ use rustc_span::Span;
1415
use crate::build::Builder;
1516
use crate::errors::{MCDCExceedsConditionLimit, MCDCExceedsDecisionDepth};
1617

17-
/// The MCDC bitmap scales exponentially (2^n) based on the number of conditions seen,
18-
/// So llvm sets a maximum value prevents the bitmap footprint from growing too large without the user's knowledge.
19-
/// This limit may be relaxed if the [upstream change](https://github.com/llvm/llvm-project/pull/82448) is merged.
20-
const MAX_CONDITIONS_IN_DECISION: usize = 6;
18+
/// LLVM uses `i16` to represent condition id. Hence `i16::MAX` is the hard limit for number of
19+
/// conditions in a decision.
20+
const MAX_CONDITIONS_IN_DECISION: usize = i16::MAX as usize;
2121

2222
/// MCDC allocates an i32 variable on stack for each depth. Ignore decisions nested too much to prevent it
2323
/// consuming excessive memory.
@@ -41,6 +41,7 @@ impl BooleanDecisionCtx {
4141
span: Span::default(),
4242
end_markers: vec![],
4343
decision_depth: 0,
44+
num_test_vectors: 0,
4445
},
4546
decision_stack: VecDeque::new(),
4647
conditions: vec![],
@@ -158,11 +159,11 @@ impl BooleanDecisionCtx {
158159
self.decision_info.end_markers.push(false_marker);
159160
}
160161

161-
self.conditions.push(MCDCBranchSpan {
162+
self.conditions.push(MCDCBranchSpan::new(
162163
span,
163164
condition_info,
164-
markers: MCDCBranchMarkers::Boolean(true_marker, false_marker),
165-
});
165+
MCDCBranchMarkers::Boolean(true_marker, false_marker),
166+
));
166167
}
167168

168169
fn is_finished(&self) -> bool {
@@ -257,9 +258,86 @@ struct MCDCTargetInfo {
257258
}
258259

259260
impl MCDCTargetInfo {
261+
fn new(decision: MCDCDecisionSpan, conditions: Vec<MCDCBranchSpan>) -> Self {
262+
let mut this = Self { decision, conditions, nested_decisions_id: vec![] };
263+
this.calc_test_vectors_index();
264+
this
265+
}
266+
260267
fn set_depth(&mut self, depth: u16) {
261268
self.decision.decision_depth = depth;
262269
}
270+
271+
// LLVM checks the executed test vector by accumulate indices of tested branches.
272+
// We calculate number of all possible test vectors of the decision and assign indices
273+
// for each branch here.
274+
// See https://discourse.llvm.org/t/rfc-coverage-new-algorithm-and-file-format-for-mc-dc/76798/ for
275+
// more details of the algorithm.
276+
// The process of this function is mostly like `TVIdxBuilder` at
277+
// https://github.com/llvm/llvm-project/blob/d594d9f7f4dc6eb748b3261917db689fdc348b96/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp#L226
278+
fn calc_test_vectors_index(&mut self) {
279+
let Self { decision, conditions, .. } = self;
280+
let mut indegree_stats = IndexVec::<ConditionId, usize>::from_elem_n(0, conditions.len());
281+
// `num_paths` is `width` described at the llvm RFC, which indicates how many paths reaching the condition.
282+
let mut num_paths_stats = IndexVec::<ConditionId, usize>::from_elem_n(0, conditions.len());
283+
let mut next_conditions = conditions
284+
.iter_mut()
285+
.map(|branch| {
286+
let ConditionInfo { condition_id, true_next_id, false_next_id } =
287+
branch.condition_info;
288+
[true_next_id, false_next_id]
289+
.into_iter()
290+
.filter_map(std::convert::identity)
291+
.for_each(|next_id| indegree_stats[next_id] += 1);
292+
(condition_id, branch)
293+
})
294+
.collect::<FxIndexMap<_, _>>();
295+
296+
let mut queue =
297+
VecDeque::from_iter(next_conditions.swap_remove(&ConditionId::START).into_iter());
298+
num_paths_stats[ConditionId::START] = 1;
299+
let mut decision_end_nodes = Vec::new();
300+
while let Some(branch) = queue.pop_front() {
301+
let MCDCBranchSpan {
302+
span: _,
303+
condition_info: ConditionInfo { condition_id, true_next_id, false_next_id },
304+
markers: _,
305+
false_index,
306+
true_index,
307+
} = branch;
308+
let this_paths_count = num_paths_stats[*condition_id];
309+
for (next, index) in [(false_next_id, false_index), (true_next_id, true_index)] {
310+
if let Some(next_id) = next {
311+
let next_paths_count = &mut num_paths_stats[*next_id];
312+
*index = *next_paths_count;
313+
*next_paths_count = next_paths_count.saturating_add(this_paths_count);
314+
let next_indegree = &mut indegree_stats[*next_id];
315+
*next_indegree -= 1;
316+
if *next_indegree == 0 {
317+
queue.push_back(next_conditions.swap_remove(next_id).expect(
318+
"conditions with non-zero indegree before must be in next_conditions",
319+
));
320+
}
321+
} else {
322+
decision_end_nodes.push((this_paths_count, *condition_id, index));
323+
}
324+
}
325+
}
326+
assert!(next_conditions.is_empty(), "the decision tree has untouched nodes");
327+
let mut cur_idx = 0;
328+
// LLVM hopes the end nodes is sorted in ascending order by `num_paths`.
329+
decision_end_nodes.sort_by_key(|(num_paths, _, _)| usize::MAX - *num_paths);
330+
for (num_paths, condition_id, index) in decision_end_nodes {
331+
assert_eq!(
332+
num_paths, num_paths_stats[condition_id],
333+
"end nodes should not be updated since they were visited"
334+
);
335+
assert_eq!(*index, usize::MAX, "end nodes should not be assigned index before");
336+
*index = cur_idx;
337+
cur_idx += num_paths;
338+
}
339+
decision.num_test_vectors = cur_idx;
340+
}
263341
}
264342

265343
#[derive(Default)]
@@ -323,7 +401,7 @@ impl MCDCInfoBuilder {
323401
}
324402
// Ignore decisions with only one condition given that mcdc for them is completely equivalent to branch coverage.
325403
2..=MAX_CONDITIONS_IN_DECISION => {
326-
let info = MCDCTargetInfo { decision, conditions, nested_decisions_id: vec![] };
404+
let info = MCDCTargetInfo::new(decision, conditions);
327405
Some(self.mcdc_targets.entry(id).or_insert(info))
328406
}
329407
_ => {

0 commit comments

Comments
 (0)