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

Rollup of 4 pull requests #130456

Merged
merged 15 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
7 changes: 6 additions & 1 deletion compiler/rustc_llvm/llvm-wrapper/LLVMWrapper.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
#include "llvm/Target/TargetMachine.h"
#include "llvm/Target/TargetOptions.h"
#include "llvm/Transforms/IPO.h"
#include "llvm/Transforms/Instrumentation.h"
#include "llvm/Transforms/Scalar.h"

#define LLVM_VERSION_GE(major, minor) \
Expand All @@ -34,6 +33,12 @@

#define LLVM_VERSION_LT(major, minor) (!LLVM_VERSION_GE((major), (minor)))

#if LLVM_VERSION_GE(20, 0)
#include "llvm/Transforms/Utils/Instrumentation.h"
#else
#include "llvm/Transforms/Instrumentation.h"
#endif

#include "llvm/IR/LegacyPassManager.h"

#include "llvm/Bitcode/BitcodeReader.h"
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
#include "llvm/TargetParser/Host.h"
#endif
#include "llvm/Support/TimeProfiler.h"
#include "llvm/Transforms/Instrumentation.h"
#include "llvm/Transforms/Instrumentation/AddressSanitizer.h"
#include "llvm/Transforms/Instrumentation/DataFlowSanitizer.h"
#if LLVM_VERSION_GE(19, 0)
Expand Down
231 changes: 109 additions & 122 deletions compiler/rustc_mir_transform/src/coverage/counters.rs

Large diffs are not rendered by default.

90 changes: 61 additions & 29 deletions compiler/rustc_mir_transform/src/coverage/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@ impl CoverageGraph {
for &bb in basic_blocks.iter() {
bb_to_bcb[bb] = Some(bcb);
}
let bcb_data = BasicCoverageBlockData::from(basic_blocks);

let is_out_summable = basic_blocks.last().map_or(false, |&bb| {
bcb_filtered_successors(mir_body[bb].terminator()).is_out_summable()
});
let bcb_data = BasicCoverageBlockData { basic_blocks, is_out_summable };
debug!("adding bcb{}: {:?}", bcb.index(), bcb_data);
bcbs.push(bcb_data);
};
Expand Down Expand Up @@ -161,23 +165,33 @@ impl CoverageGraph {
self.dominators.as_ref().unwrap().cmp_in_dominator_order(a, b)
}

/// Returns true if the given node has 2 or more in-edges, i.e. 2 or more
/// predecessors.
///
/// This property is interesting to code that assigns counters to nodes and
/// edges, because if a node _doesn't_ have multiple in-edges, then there's
/// no benefit in having a separate counter for its in-edge, because it
/// would have the same value as the node's own counter.
///
/// FIXME: That assumption might not be true for [`TerminatorKind::Yield`]?
#[inline(always)]
pub(crate) fn bcb_has_multiple_in_edges(&self, bcb: BasicCoverageBlock) -> bool {
// Even though bcb0 conceptually has an extra virtual in-edge due to
// being the entry point, we've already asserted that it has no _other_
// in-edges, so there's no possibility of it having _multiple_ in-edges.
// (And since its virtual in-edge doesn't exist in the graph, that edge
// can't have a separate counter anyway.)
self.predecessors[bcb].len() > 1
/// Returns the source of this node's sole in-edge, if it has exactly one.
/// That edge can be assumed to have the same execution count as the node
/// itself (in the absence of panics).
pub(crate) fn sole_predecessor(
&self,
to_bcb: BasicCoverageBlock,
) -> Option<BasicCoverageBlock> {
// Unlike `simple_successor`, there is no need for extra checks here.
if let &[from_bcb] = self.predecessors[to_bcb].as_slice() { Some(from_bcb) } else { None }
}

/// Returns the target of this node's sole out-edge, if it has exactly
/// one, but only if that edge can be assumed to have the same execution
/// count as the node itself (in the absence of panics).
pub(crate) fn simple_successor(
&self,
from_bcb: BasicCoverageBlock,
) -> Option<BasicCoverageBlock> {
// If a node's count is the sum of its out-edges, and it has exactly
// one out-edge, then that edge has the same count as the node.
if self.bcbs[from_bcb].is_out_summable
&& let &[to_bcb] = self.successors[from_bcb].as_slice()
{
Some(to_bcb)
} else {
None
}
}
}

Expand Down Expand Up @@ -266,14 +280,16 @@ rustc_index::newtype_index! {
#[derive(Debug, Clone)]
pub(crate) struct BasicCoverageBlockData {
pub(crate) basic_blocks: Vec<BasicBlock>,

/// If true, this node's execution count can be assumed to be the sum of the
/// execution counts of all of its **out-edges** (assuming no panics).
///
/// Notably, this is false for a node ending with [`TerminatorKind::Yield`],
/// because the yielding coroutine might not be resumed.
pub(crate) is_out_summable: bool,
}

impl BasicCoverageBlockData {
fn from(basic_blocks: Vec<BasicBlock>) -> Self {
assert!(basic_blocks.len() > 0);
Self { basic_blocks }
}

#[inline(always)]
pub(crate) fn leader_bb(&self) -> BasicBlock {
self.basic_blocks[0]
Expand All @@ -295,13 +311,27 @@ enum CoverageSuccessors<'a> {
Chainable(BasicBlock),
/// The block cannot be combined into the same BCB as its successor(s).
NotChainable(&'a [BasicBlock]),
/// Yield terminators are not chainable, and their execution count can also
/// differ from the execution count of their out-edge.
Yield(BasicBlock),
}

impl CoverageSuccessors<'_> {
fn is_chainable(&self) -> bool {
match self {
Self::Chainable(_) => true,
Self::NotChainable(_) => false,
Self::Yield(_) => false,
}
}

/// Returns true if the terminator itself is assumed to have the same
/// execution count as the sum of its out-edges (assuming no panics).
fn is_out_summable(&self) -> bool {
match self {
Self::Chainable(_) => true,
Self::NotChainable(_) => true,
Self::Yield(_) => false,
}
}
}
Expand All @@ -312,7 +342,9 @@ impl IntoIterator for CoverageSuccessors<'_> {

fn into_iter(self) -> Self::IntoIter {
match self {
Self::Chainable(bb) => Some(bb).into_iter().chain((&[]).iter().copied()),
Self::Chainable(bb) | Self::Yield(bb) => {
Some(bb).into_iter().chain((&[]).iter().copied())
}
Self::NotChainable(bbs) => None.into_iter().chain(bbs.iter().copied()),
}
}
Expand All @@ -331,7 +363,7 @@ fn bcb_filtered_successors<'a, 'tcx>(terminator: &'a Terminator<'tcx>) -> Covera

// A yield terminator has exactly 1 successor, but should not be chained,
// because its resume edge has a different execution count.
Yield { ref resume, .. } => CoverageSuccessors::NotChainable(std::slice::from_ref(resume)),
Yield { resume, .. } => CoverageSuccessors::Yield(resume),

// These terminators have exactly one coverage-relevant successor,
// and can be chained into it.
Expand All @@ -341,15 +373,15 @@ fn bcb_filtered_successors<'a, 'tcx>(terminator: &'a Terminator<'tcx>) -> Covera
| FalseUnwind { real_target: target, .. }
| Goto { target } => CoverageSuccessors::Chainable(target),

// A call terminator can normally be chained, except when they have no
// successor because they are known to diverge.
// A call terminator can normally be chained, except when it has no
// successor because it is known to diverge.
Call { target: maybe_target, .. } => match maybe_target {
Some(target) => CoverageSuccessors::Chainable(target),
None => CoverageSuccessors::NotChainable(&[]),
},

// An inline asm terminator can normally be chained, except when it diverges or uses asm
// goto.
// An inline asm terminator can normally be chained, except when it
// diverges or uses asm goto.
InlineAsm { ref targets, .. } => {
if let [target] = targets[..] {
CoverageSuccessors::Chainable(target)
Expand Down
2 changes: 0 additions & 2 deletions library/std/src/sync/lazy_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,6 @@ union Data<T, F> {
///
/// // The `String` is built, stored in the `LazyLock`, and returned as `&String`.
/// let _ = &*DEEP_THOUGHT;
/// // The `String` is retrieved from the `LazyLock` and returned as `&String`.
/// let _ = &*DEEP_THOUGHT;
/// ```
///
/// Initialize fields with `LazyLock`.
Expand Down
Loading
Loading