From 6d69eb1f2e3d130a3e01412958f2387c1ad3b047 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 29 Oct 2023 20:50:47 +1100 Subject: [PATCH 1/3] coverage: Replace manual debug indents with nested tracing spans --- .../src/coverage/counters.rs | 84 +++++-------------- 1 file changed, 19 insertions(+), 65 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index d07f59bc72af5..2e3e560975f0c 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -12,8 +12,6 @@ use rustc_middle::mir::coverage::*; use std::fmt::{self, Debug}; -const NESTED_INDENT: &str = " "; - /// The coverage counter or counter expression associated with a particular /// BCB node or BCB edge. #[derive(Clone)] @@ -347,22 +345,17 @@ impl<'a> MakeBcbCounters<'a> { } fn get_or_make_counter_operand(&mut self, bcb: BasicCoverageBlock) -> Result { - self.recursive_get_or_make_counter_operand(bcb, 1) + self.recursive_get_or_make_counter_operand(bcb) } + #[instrument(level = "debug", skip(self))] fn recursive_get_or_make_counter_operand( &mut self, bcb: BasicCoverageBlock, - debug_indent_level: usize, ) -> Result { // If the BCB already has a counter, return it. if let Some(counter_kind) = &self.coverage_counters.bcb_counters[bcb] { - debug!( - "{}{:?} already has a counter: {:?}", - NESTED_INDENT.repeat(debug_indent_level), - bcb, - counter_kind, - ); + debug!("{bcb:?} already has a counter: {counter_kind:?}"); return Ok(counter_kind.as_term()); } @@ -373,20 +366,12 @@ impl<'a> MakeBcbCounters<'a> { if one_path_to_target || self.bcb_predecessors(bcb).contains(&bcb) { let counter_kind = self.coverage_counters.make_counter(); if one_path_to_target { - debug!( - "{}{:?} gets a new counter: {:?}", - NESTED_INDENT.repeat(debug_indent_level), - bcb, - counter_kind, - ); + debug!("{bcb:?} gets a new counter: {counter_kind:?}"); } else { debug!( - "{}{:?} has itself as its own predecessor. It can't be part of its own \ - Expression sum, so it will get its own new counter: {:?}. (Note, the compiled \ - code will generate an infinite loop.)", - NESTED_INDENT.repeat(debug_indent_level), - bcb, - counter_kind, + "{bcb:?} has itself as its own predecessor. It can't be part of its own \ + Expression sum, so it will get its own new counter: {counter_kind:?}. \ + (Note, the compiled code will generate an infinite loop.)", ); } return self.coverage_counters.set_bcb_counter(bcb, counter_kind); @@ -397,23 +382,13 @@ impl<'a> MakeBcbCounters<'a> { // counters for those incoming edges first, then call `make_expression()` to sum them up, // with additional intermediate expressions as needed. let mut predecessors = self.bcb_predecessors(bcb).to_owned().into_iter(); - debug!( - "{}{:?} has multiple incoming edges and will get an expression that sums them up...", - NESTED_INDENT.repeat(debug_indent_level), - bcb, - ); - let first_edge_counter_operand = self.recursive_get_or_make_edge_counter_operand( - predecessors.next().unwrap(), - bcb, - debug_indent_level + 1, - )?; + debug!("{bcb:?} has multiple incoming edges and will need a sum-up expression"); + let first_edge_counter_operand = + self.recursive_get_or_make_edge_counter_operand(predecessors.next().unwrap(), bcb)?; let mut some_sumup_edge_counter_operand = None; for predecessor in predecessors { - let edge_counter_operand = self.recursive_get_or_make_edge_counter_operand( - predecessor, - bcb, - debug_indent_level + 1, - )?; + let edge_counter_operand = + self.recursive_get_or_make_edge_counter_operand(predecessor, bcb)?; if let Some(sumup_edge_counter_operand) = some_sumup_edge_counter_operand.replace(edge_counter_operand) { @@ -422,11 +397,7 @@ impl<'a> MakeBcbCounters<'a> { Op::Add, edge_counter_operand, ); - debug!( - "{}new intermediate expression: {:?}", - NESTED_INDENT.repeat(debug_indent_level), - intermediate_expression - ); + debug!("new intermediate expression: {intermediate_expression:?}"); let intermediate_expression_operand = intermediate_expression.as_term(); some_sumup_edge_counter_operand.replace(intermediate_expression_operand); } @@ -436,12 +407,7 @@ impl<'a> MakeBcbCounters<'a> { Op::Add, some_sumup_edge_counter_operand.unwrap(), ); - debug!( - "{}{:?} gets a new counter (sum of predecessor counters): {:?}", - NESTED_INDENT.repeat(debug_indent_level), - bcb, - counter_kind - ); + debug!("{bcb:?} gets a new counter (sum of predecessor counters): {counter_kind:?}"); self.coverage_counters.set_bcb_counter(bcb, counter_kind) } @@ -450,45 +416,33 @@ impl<'a> MakeBcbCounters<'a> { from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, ) -> Result { - self.recursive_get_or_make_edge_counter_operand(from_bcb, to_bcb, 1) + self.recursive_get_or_make_edge_counter_operand(from_bcb, to_bcb) } + #[instrument(level = "debug", skip(self))] fn recursive_get_or_make_edge_counter_operand( &mut self, from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, - debug_indent_level: usize, ) -> Result { // If the source BCB has only one successor (assumed to be the given target), an edge // counter is unnecessary. Just get or make a counter for the source BCB. let successors = self.bcb_successors(from_bcb).iter(); if successors.len() == 1 { - return self.recursive_get_or_make_counter_operand(from_bcb, debug_indent_level + 1); + return self.recursive_get_or_make_counter_operand(from_bcb); } // If the edge already has a counter, return it. if let Some(counter_kind) = self.coverage_counters.bcb_edge_counters.get(&(from_bcb, to_bcb)) { - debug!( - "{}Edge {:?}->{:?} already has a counter: {:?}", - NESTED_INDENT.repeat(debug_indent_level), - from_bcb, - to_bcb, - counter_kind - ); + debug!("Edge {from_bcb:?}->{to_bcb:?} already has a counter: {counter_kind:?}"); return Ok(counter_kind.as_term()); } // Make a new counter to count this edge. let counter_kind = self.coverage_counters.make_counter(); - debug!( - "{}Edge {:?}->{:?} gets a new counter: {:?}", - NESTED_INDENT.repeat(debug_indent_level), - from_bcb, - to_bcb, - counter_kind - ); + debug!("Edge {from_bcb:?}->{to_bcb:?} gets a new counter: {counter_kind:?}"); self.coverage_counters.set_bcb_edge_counter(from_bcb, to_bcb, counter_kind) } From 2f1be08473e07e15d848b21e159be6729ffd90d2 Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 29 Oct 2023 20:53:17 +1100 Subject: [PATCH 2/3] coverage: Inline the "recursive" worker methods for assigning counters Now that we don't manually pass around indent levels, there's no need for these worker methods to exist separately from their main callers. --- .../src/coverage/counters.rs | 26 ++++--------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 2e3e560975f0c..8909de143335c 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -344,15 +344,8 @@ impl<'a> MakeBcbCounters<'a> { Ok(()) } - fn get_or_make_counter_operand(&mut self, bcb: BasicCoverageBlock) -> Result { - self.recursive_get_or_make_counter_operand(bcb) - } - #[instrument(level = "debug", skip(self))] - fn recursive_get_or_make_counter_operand( - &mut self, - bcb: BasicCoverageBlock, - ) -> Result { + fn get_or_make_counter_operand(&mut self, bcb: BasicCoverageBlock) -> Result { // If the BCB already has a counter, return it. if let Some(counter_kind) = &self.coverage_counters.bcb_counters[bcb] { debug!("{bcb:?} already has a counter: {counter_kind:?}"); @@ -384,11 +377,10 @@ impl<'a> MakeBcbCounters<'a> { let mut predecessors = self.bcb_predecessors(bcb).to_owned().into_iter(); debug!("{bcb:?} has multiple incoming edges and will need a sum-up expression"); let first_edge_counter_operand = - self.recursive_get_or_make_edge_counter_operand(predecessors.next().unwrap(), bcb)?; + self.get_or_make_edge_counter_operand(predecessors.next().unwrap(), bcb)?; let mut some_sumup_edge_counter_operand = None; for predecessor in predecessors { - let edge_counter_operand = - self.recursive_get_or_make_edge_counter_operand(predecessor, bcb)?; + let edge_counter_operand = self.get_or_make_edge_counter_operand(predecessor, bcb)?; if let Some(sumup_edge_counter_operand) = some_sumup_edge_counter_operand.replace(edge_counter_operand) { @@ -411,16 +403,8 @@ impl<'a> MakeBcbCounters<'a> { self.coverage_counters.set_bcb_counter(bcb, counter_kind) } - fn get_or_make_edge_counter_operand( - &mut self, - from_bcb: BasicCoverageBlock, - to_bcb: BasicCoverageBlock, - ) -> Result { - self.recursive_get_or_make_edge_counter_operand(from_bcb, to_bcb) - } - #[instrument(level = "debug", skip(self))] - fn recursive_get_or_make_edge_counter_operand( + fn get_or_make_edge_counter_operand( &mut self, from_bcb: BasicCoverageBlock, to_bcb: BasicCoverageBlock, @@ -429,7 +413,7 @@ impl<'a> MakeBcbCounters<'a> { // counter is unnecessary. Just get or make a counter for the source BCB. let successors = self.bcb_successors(from_bcb).iter(); if successors.len() == 1 { - return self.recursive_get_or_make_counter_operand(from_bcb); + return self.get_or_make_counter_operand(from_bcb); } // If the edge already has a counter, return it. From 10c4734c79c7fc2cad772f0f013032ffeecbce1b Mon Sep 17 00:00:00 2001 From: Zalathar Date: Sun, 29 Oct 2023 21:23:47 +1100 Subject: [PATCH 3/3] coverage: Use a tracing span to group the parts of a sum-up expression --- compiler/rustc_mir_transform/src/coverage/counters.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_mir_transform/src/coverage/counters.rs b/compiler/rustc_mir_transform/src/coverage/counters.rs index 8909de143335c..dcf603a4f77c8 100644 --- a/compiler/rustc_mir_transform/src/coverage/counters.rs +++ b/compiler/rustc_mir_transform/src/coverage/counters.rs @@ -374,8 +374,9 @@ impl<'a> MakeBcbCounters<'a> { // counters and/or expressions of its incoming edges. This will recursively get or create // counters for those incoming edges first, then call `make_expression()` to sum them up, // with additional intermediate expressions as needed. + let _sumup_debug_span = debug_span!("(preparing sum-up expression)").entered(); + let mut predecessors = self.bcb_predecessors(bcb).to_owned().into_iter(); - debug!("{bcb:?} has multiple incoming edges and will need a sum-up expression"); let first_edge_counter_operand = self.get_or_make_edge_counter_operand(predecessors.next().unwrap(), bcb)?; let mut some_sumup_edge_counter_operand = None; @@ -399,6 +400,8 @@ impl<'a> MakeBcbCounters<'a> { Op::Add, some_sumup_edge_counter_operand.unwrap(), ); + drop(_sumup_debug_span); + debug!("{bcb:?} gets a new counter (sum of predecessor counters): {counter_kind:?}"); self.coverage_counters.set_bcb_counter(bcb, counter_kind) }