Skip to content

Commit

Permalink
Auto merge of #113339 - lqd:respect-filters, r=tmiasko
Browse files Browse the repository at this point in the history
Filter out short-lived LLVM diagnostics before they reach the rustc handler

During profiling I saw remark passes being unconditionally enabled: for example `Machine Optimization Remark Emitter`.

The diagnostic remarks enabled by default are [from missed optimizations and opt analyses](#113339 (comment)). They are created by LLVM, passed to the diagnostic handler on the C++ side, emitted to rust, where they are unpacked, C++ strings are converted to rust, etc.

Then they are discarded in the vast majority of the time (i.e. unless some kind of `-Cremark` has enabled some of these passes' output to be printed).

These unneeded allocations are very short-lived, basically only lasting between the LLVM pass emitting them and the rust handler where they are discarded. So it doesn't hugely impact max-rss, and is only a slight reduction in instruction count (cachegrind reports a reduction between 0.3% and 0.5%) _on linux_. It's possible that targets without `jemalloc` or with a worse allocator, may optimize these less.

It is however significant in the aggregate, looking at the total number of allocated bytes:
- it's the biggest source of allocations according to dhat, on the benchmarks I've tried e.g. `syn` or `cargo`
- allocations on `syn` are reduced by 440MB, 17% (from 2440722647 bytes total, to 2030461328 bytes)
- allocations on `cargo` are reduced by 6.6GB, 19% (from 35371886402 bytes total, to 28723987743 bytes)

Some of these diagnostics objects [are allocated in LLVM](#113339 (comment)) *before* they're emitted to our diagnostic handler, where they'll be filtered out. So we could remove those in the future, but that will require changing a few LLVM call-sites upstream, so I left a FIXME.
  • Loading branch information
bors committed Aug 1, 2023
2 parents d12c6e9 + ca5a383 commit f77c624
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 27 deletions.
39 changes: 16 additions & 23 deletions compiler/rustc_codegen_llvm/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,29 +381,22 @@ unsafe extern "C" fn diagnostic_handler(info: &DiagnosticInfo, user: *mut c_void
}

llvm::diagnostic::Optimization(opt) => {
let enabled = match cgcx.remark {
Passes::All => true,
Passes::Some(ref v) => v.iter().any(|s| *s == opt.pass_name),
};

if enabled {
diag_handler.emit_note(FromLlvmOptimizationDiag {
filename: &opt.filename,
line: opt.line,
column: opt.column,
pass_name: &opt.pass_name,
kind: match opt.kind {
OptimizationDiagnosticKind::OptimizationRemark => "success",
OptimizationDiagnosticKind::OptimizationMissed
| OptimizationDiagnosticKind::OptimizationFailure => "missed",
OptimizationDiagnosticKind::OptimizationAnalysis
| OptimizationDiagnosticKind::OptimizationAnalysisFPCommute
| OptimizationDiagnosticKind::OptimizationAnalysisAliasing => "analysis",
OptimizationDiagnosticKind::OptimizationRemarkOther => "other",
},
message: &opt.message,
});
}
diag_handler.emit_note(FromLlvmOptimizationDiag {
filename: &opt.filename,
line: opt.line,
column: opt.column,
pass_name: &opt.pass_name,
kind: match opt.kind {
OptimizationDiagnosticKind::OptimizationRemark => "success",
OptimizationDiagnosticKind::OptimizationMissed
| OptimizationDiagnosticKind::OptimizationFailure => "missed",
OptimizationDiagnosticKind::OptimizationAnalysis
| OptimizationDiagnosticKind::OptimizationAnalysisFPCommute
| OptimizationDiagnosticKind::OptimizationAnalysisAliasing => "analysis",
OptimizationDiagnosticKind::OptimizationRemarkOther => "other",
},
message: &opt.message,
});
}
llvm::diagnostic::PGO(diagnostic_ref) | llvm::diagnostic::Linker(diagnostic_ref) => {
let message = llvm::build_string(|s| {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/llvm/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use libc::c_uint;
use super::{DiagnosticInfo, SMDiagnostic};
use rustc_span::InnerSpan;

#[derive(Copy, Clone)]
#[derive(Copy, Clone, Debug)]
pub enum OptimizationDiagnosticKind {
OptimizationRemark,
OptimizationMissed,
Expand Down
13 changes: 10 additions & 3 deletions compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1892,12 +1892,19 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler(
LlvmRemarkStreamer(std::move(LlvmRemarkStreamer)) {}

virtual bool handleDiagnostics(const DiagnosticInfo &DI) override {
if (this->LlvmRemarkStreamer) {
if (auto *OptDiagBase = dyn_cast<DiagnosticInfoOptimizationBase>(&DI)) {
if (OptDiagBase->isEnabled()) {
// If this diagnostic is one of the optimization remark kinds, we can check if it's enabled
// before emitting it. This can avoid many short-lived allocations when unpacking the
// diagnostic and converting its various C++ strings into rust strings.
// FIXME: some diagnostic infos still allocate before we get here, and avoiding that would be
// good in the future. That will require changing a few call sites in LLVM.
if (auto *OptDiagBase = dyn_cast<DiagnosticInfoOptimizationBase>(&DI)) {
if (OptDiagBase->isEnabled()) {
if (this->LlvmRemarkStreamer) {
this->LlvmRemarkStreamer->emit(*OptDiagBase);
return true;
}
} else {
return true;
}
}
if (DiagnosticHandlerCallback) {
Expand Down

0 comments on commit f77c624

Please sign in to comment.