Skip to content

Commit

Permalink
Rollup merge of #131829 - Zalathar:goodbye-zprofile, r=chenyukang
Browse files Browse the repository at this point in the history
Remove support for `-Zprofile` (gcov-style coverage instrumentation)

Tracking issue: #42524

MCP: rust-lang/compiler-team#798

---

This PR removes the unstable `-Zprofile` flag, which enables ”gcov-style” coverage instrumentation, along with its associated `-Zprofile-emit` configuration flag.

(The profile flag predates and is almost entirely separate from the stable `-Cinstrument-coverage` flag.)

Notably, the `-Zprofile` flag:
- Is largely untested in-tree, having only one run-make test that does not check whether its output is correct or useful.
- Has no known maintainer.
- Has seen no push towards stabilization.
- Has at least one severe regression reported in 2022 that apparently remains unaddressed.
  - #100125
- Is confusingly named, since it appears to be more about coverage than performance profiling, and has nothing to do with PGO.
- Is fundamentally limited by relying on counters auto-inserted by LLVM, with no knowledge of Rust beyond debuginfo.
  • Loading branch information
GuillaumeGomez authored Nov 1, 2024
2 parents 5ca0e9f + ce3e14a commit 526c67f
Show file tree
Hide file tree
Showing 15 changed files with 10 additions and 146 deletions.
5 changes: 0 additions & 5 deletions compiler/rustc_codegen_llvm/src/attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,6 @@ fn probestack_attr<'ll>(cx: &CodegenCx<'ll, '_>) -> Option<&'ll Attribute> {
return None;
}

// probestack doesn't play nice either with gcov profiling.
if cx.sess().opts.unstable_opts.profile {
return None;
}

let attr_value = match cx.sess().target.stack_probes {
StackProbeType::None => return None,
// Request LLVM to generate the probes inline. If the given LLVM version does not support
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_codegen_llvm/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,6 @@ pub(crate) unsafe fn llvm_optimize(
pgo_use_path.as_ref().map_or(std::ptr::null(), |s| s.as_ptr()),
config.instrument_coverage,
instr_profile_output_path.as_ref().map_or(std::ptr::null(), |s| s.as_ptr()),
config.instrument_gcov,
pgo_sample_use_path.as_ref().map_or(std::ptr::null(), |s| s.as_ptr()),
config.debug_info_for_profiling,
llvm_selfprofiler,
Expand Down
26 changes: 0 additions & 26 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::{iter, ptr};
use libc::{c_char, c_longlong, c_uint};
use rustc_codegen_ssa::debuginfo::type_names::{VTableNameKind, cpp_like_debuginfo};
use rustc_codegen_ssa::traits::*;
use rustc_fs_util::path_to_c_string;
use rustc_hir::def::{CtorKind, DefKind};
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
use rustc_middle::bug;
Expand Down Expand Up @@ -979,33 +978,8 @@ pub(crate) fn build_compile_unit_di_node<'ll, 'tcx>(
debug_name_table_kind,
);

if tcx.sess.opts.unstable_opts.profile {
let default_gcda_path = &output_filenames.with_extension("gcda");
let gcda_path =
tcx.sess.opts.unstable_opts.profile_emit.as_ref().unwrap_or(default_gcda_path);

let gcov_cu_info = [
path_to_mdstring(debug_context.llcontext, &output_filenames.with_extension("gcno")),
path_to_mdstring(debug_context.llcontext, gcda_path),
unit_metadata,
];
let gcov_metadata = llvm::LLVMMDNodeInContext2(
debug_context.llcontext,
gcov_cu_info.as_ptr(),
gcov_cu_info.len(),
);
let val = llvm::LLVMMetadataAsValue(debug_context.llcontext, gcov_metadata);

llvm::LLVMAddNamedMetadataOperand(debug_context.llmod, c"llvm.gcov".as_ptr(), val);
}

return unit_metadata;
};

fn path_to_mdstring<'ll>(llcx: &'ll llvm::Context, path: &Path) -> &'ll llvm::Metadata {
let path_str = path_to_c_string(path);
unsafe { llvm::LLVMMDStringInContext2(llcx, path_str.as_ptr(), path_str.as_bytes().len()) }
}
}

/// Creates a `DW_TAG_member` entry inside the DIE represented by the given `type_di_node`.
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_codegen_llvm/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ const DW_TAG_arg_variable: c_uint = 0x101;

/// A context object for maintaining all state needed by the debuginfo module.
pub(crate) struct CodegenUnitDebugContext<'ll, 'tcx> {
llcontext: &'ll llvm::Context,
llmod: &'ll llvm::Module,
builder: &'ll mut DIBuilder<'ll>,
created_files: RefCell<UnordMap<Option<(StableSourceFileId, SourceFileHash)>, &'ll DIFile>>,
Expand All @@ -78,9 +77,7 @@ impl<'ll, 'tcx> CodegenUnitDebugContext<'ll, 'tcx> {
debug!("CodegenUnitDebugContext::new");
let builder = unsafe { llvm::LLVMRustDIBuilderCreate(llmod) };
// DIBuilder inherits context from the module, so we'd better use the same one
let llcontext = unsafe { llvm::LLVMGetModuleContext(llmod) };
CodegenUnitDebugContext {
llcontext,
llmod,
builder,
created_files: Default::default(),
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2269,7 +2269,6 @@ unsafe extern "C" {
PGOUsePath: *const c_char,
InstrumentCoverage: bool,
InstrProfileOutput: *const c_char,
InstrumentGCOV: bool,
PGOSampleUsePath: *const c_char,
DebugInfoForProfiling: bool,
llvm_selfprofiler: *mut c_void,
Expand Down
25 changes: 4 additions & 21 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ pub struct ModuleConfig {
pub pgo_sample_use: Option<PathBuf>,
pub debug_info_for_profiling: bool,
pub instrument_coverage: bool,
pub instrument_gcov: bool,

pub sanitizer: SanitizerSet,
pub sanitizer_recover: SanitizerSet,
Expand Down Expand Up @@ -123,12 +122,7 @@ pub struct ModuleConfig {
}

impl ModuleConfig {
fn new(
kind: ModuleKind,
tcx: TyCtxt<'_>,
no_builtins: bool,
is_compiler_builtins: bool,
) -> ModuleConfig {
fn new(kind: ModuleKind, tcx: TyCtxt<'_>, no_builtins: bool) -> ModuleConfig {
// If it's a regular module, use `$regular`, otherwise use `$other`.
// `$regular` and `$other` are evaluated lazily.
macro_rules! if_regular {
Expand Down Expand Up @@ -189,13 +183,6 @@ impl ModuleConfig {
pgo_sample_use: if_regular!(sess.opts.unstable_opts.profile_sample_use.clone(), None),
debug_info_for_profiling: sess.opts.unstable_opts.debug_info_for_profiling,
instrument_coverage: if_regular!(sess.instrument_coverage(), false),
instrument_gcov: if_regular!(
// compiler_builtins overrides the codegen-units settings,
// which is incompatible with -Zprofile which requires that
// only a single codegen unit is used per crate.
sess.opts.unstable_opts.profile && !is_compiler_builtins,
false
),

sanitizer: if_regular!(sess.opts.unstable_opts.sanitizer, SanitizerSet::empty()),
sanitizer_dataflow_abilist: if_regular!(
Expand Down Expand Up @@ -473,16 +460,12 @@ pub(crate) fn start_async_codegen<B: ExtraBackendMethods>(

let crate_attrs = tcx.hir().attrs(rustc_hir::CRATE_HIR_ID);
let no_builtins = attr::contains_name(crate_attrs, sym::no_builtins);
let is_compiler_builtins = attr::contains_name(crate_attrs, sym::compiler_builtins);

let crate_info = CrateInfo::new(tcx, target_cpu);

let regular_config =
ModuleConfig::new(ModuleKind::Regular, tcx, no_builtins, is_compiler_builtins);
let metadata_config =
ModuleConfig::new(ModuleKind::Metadata, tcx, no_builtins, is_compiler_builtins);
let allocator_config =
ModuleConfig::new(ModuleKind::Allocator, tcx, no_builtins, is_compiler_builtins);
let regular_config = ModuleConfig::new(ModuleKind::Regular, tcx, no_builtins);
let metadata_config = ModuleConfig::new(ModuleKind::Metadata, tcx, no_builtins);
let allocator_config = ModuleConfig::new(ModuleKind::Allocator, tcx, no_builtins);

let (shared_emitter, shared_emitter_main) = SharedEmitter::new();
let (codegen_worker_send, codegen_worker_receive) = channel();
Expand Down
2 changes: 0 additions & 2 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -832,8 +832,6 @@ fn test_unstable_options_tracking_hash() {
tracked!(polonius, Polonius::Legacy);
tracked!(precise_enum_drop_elaboration, false);
tracked!(print_fuel, Some("abc".to_string()));
tracked!(profile, true);
tracked!(profile_emit, Some(PathBuf::from("abc")));
tracked!(profile_sample_use, Some(PathBuf::from("abc")));
tracked!(profiler_runtime, "abc".to_string());
tracked!(regparm, Some(3));
Expand Down
13 changes: 2 additions & 11 deletions compiler/rustc_llvm/llvm-wrapper/PassWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
#if LLVM_VERSION_GE(19, 0)
#include "llvm/Support/PGOOptions.h"
#endif
#include "llvm/Transforms/Instrumentation/GCOVProfiler.h"
#include "llvm/Transforms/Instrumentation/HWAddressSanitizer.h"
#include "llvm/Transforms/Instrumentation/InstrProfiling.h"
#include "llvm/Transforms/Instrumentation/MemorySanitizer.h"
Expand Down Expand Up @@ -714,9 +713,8 @@ extern "C" LLVMRustResult LLVMRustOptimize(
bool SLPVectorize, bool LoopVectorize, bool DisableSimplifyLibCalls,
bool EmitLifetimeMarkers, LLVMRustSanitizerOptions *SanitizerOptions,
const char *PGOGenPath, const char *PGOUsePath, bool InstrumentCoverage,
const char *InstrProfileOutput, bool InstrumentGCOV,
const char *PGOSampleUsePath, bool DebugInfoForProfiling,
void *LlvmSelfProfiler,
const char *InstrProfileOutput, const char *PGOSampleUsePath,
bool DebugInfoForProfiling, void *LlvmSelfProfiler,
LLVMRustSelfProfileBeforePassCallback BeforePassCallback,
LLVMRustSelfProfileAfterPassCallback AfterPassCallback,
const char *ExtraPasses, size_t ExtraPassesLen, const char *LLVMPlugins,
Expand Down Expand Up @@ -847,13 +845,6 @@ extern "C" LLVMRustResult LLVMRustOptimize(
});
}

if (InstrumentGCOV) {
PipelineStartEPCallbacks.push_back(
[](ModulePassManager &MPM, OptimizationLevel Level) {
MPM.addPass(GCOVProfilerPass(GCOVOptions::getDefault()));
});
}

if (InstrumentCoverage) {
PipelineStartEPCallbacks.push_back(
[InstrProfileOutput](ModulePassManager &MPM, OptimizationLevel Level) {
Expand Down
4 changes: 1 addition & 3 deletions compiler/rustc_metadata/src/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -778,9 +778,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {

fn inject_profiler_runtime(&mut self, krate: &ast::Crate) {
if self.sess.opts.unstable_opts.no_profiler_runtime
|| !(self.sess.instrument_coverage()
|| self.sess.opts.unstable_opts.profile
|| self.sess.opts.cg.profile_generate.enabled())
|| !(self.sess.instrument_coverage() || self.sess.opts.cg.profile_generate.enabled())
{
return;
}
Expand Down
14 changes: 1 addition & 13 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2453,7 +2453,7 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M
let output_types = parse_output_types(early_dcx, &unstable_opts, matches);

let mut cg = CodegenOptions::build(early_dcx, matches);
let (disable_local_thinlto, mut codegen_units) = should_override_cgus_and_disable_thinlto(
let (disable_local_thinlto, codegen_units) = should_override_cgus_and_disable_thinlto(
early_dcx,
&output_types,
matches,
Expand All @@ -2476,18 +2476,6 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M

let assert_incr_state = parse_assert_incr_state(early_dcx, &unstable_opts.assert_incr_state);

if unstable_opts.profile && incremental.is_some() {
early_dcx.early_fatal("can't instrument with gcov profiling when compiling incrementally");
}
if unstable_opts.profile {
match codegen_units {
Some(1) => {}
None => codegen_units = Some(1),
Some(_) => early_dcx
.early_fatal("can't instrument with gcov profiling with multiple codegen units"),
}
}

if cg.profile_generate.enabled() && cg.profile_use.is_some() {
early_dcx.early_fatal("options `-C profile-generate` and `-C profile-use` are exclusive");
}
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1985,13 +1985,8 @@ options! {
proc_macro_execution_strategy: ProcMacroExecutionStrategy = (ProcMacroExecutionStrategy::SameThread,
parse_proc_macro_execution_strategy, [UNTRACKED],
"how to run proc-macro code (default: same-thread)"),
profile: bool = (false, parse_bool, [TRACKED],
"insert profiling code (default: no)"),
profile_closures: bool = (false, parse_no_flag, [UNTRACKED],
"profile size of closures"),
profile_emit: Option<PathBuf> = (None, parse_opt_pathbuf, [TRACKED],
"file path to emit profiling data at runtime when using 'profile' \
(default based on relative source path)"),
profile_sample_use: Option<PathBuf> = (None, parse_opt_pathbuf, [TRACKED],
"use the given `.prof` file for sampled profile-guided optimization (also known as AutoFDO)"),
profiler_runtime: String = (String::from("profiler_builtins"), parse_string, [TRACKED],
Expand Down
8 changes: 2 additions & 6 deletions src/doc/rustc/src/instrument-coverage.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,8 @@

## Introduction

The Rust compiler includes two code coverage implementations:

- A GCC-compatible, gcov-based coverage implementation, enabled with `-Z profile`, which derives coverage data based on DebugInfo.
- A source-based code coverage implementation, enabled with `-C instrument-coverage`, which uses LLVM's native, efficient coverage instrumentation to generate very precise coverage data.

This document describes how to enable and use the LLVM instrumentation-based coverage, via the `-C instrument-coverage` compiler flag.
This document describes how to enable and use LLVM instrumentation-based coverage,
via the `-C instrument-coverage` compiler flag.

## How it works

Expand Down
27 changes: 0 additions & 27 deletions src/doc/unstable-book/src/compiler-flags/profile.md

This file was deleted.

21 changes: 0 additions & 21 deletions tests/run-make/profile/rmake.rs

This file was deleted.

1 change: 0 additions & 1 deletion tests/run-make/profile/test.rs

This file was deleted.

0 comments on commit 526c67f

Please sign in to comment.