Skip to content

Commit

Permalink
Auto merge of rust-lang#130456 - matthiaskrgr:rollup-h2qvk1f, r=matth…
Browse files Browse the repository at this point in the history
…iaskrgr

Rollup of 4 pull requests

Successful merges:

 - rust-lang#130380 (coverage: Clarify some parts of coverage counter creation)
 - rust-lang#130427 (run_make_support: rectify symlink handling)
 - rust-lang#130447 (rustc_llvm: update for llvm/llvm-project@2ae968a0d9fb61606b020e898d88…)
 - rust-lang#130448 (fix: Remove duplicate `LazyLock` example.)

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Sep 17, 2024
2 parents e2dc1a1 + 558b302 commit c8dff28
Show file tree
Hide file tree
Showing 10 changed files with 334 additions and 235 deletions.
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

0 comments on commit c8dff28

Please sign in to comment.