Skip to content

Commit f77c624

Browse files
committed
Auto merge of rust-lang#113339 - lqd:respect-filters, r=tmiasko
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](rust-lang#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](rust-lang#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.
2 parents d12c6e9 + ca5a383 commit f77c624

File tree

3 files changed

+27
-27
lines changed

3 files changed

+27
-27
lines changed

Diff for: compiler/rustc_codegen_llvm/src/back/write.rs

+16-23
Original file line numberDiff line numberDiff line change
@@ -381,29 +381,22 @@ unsafe extern "C" fn diagnostic_handler(info: &DiagnosticInfo, user: *mut c_void
381381
}
382382

383383
llvm::diagnostic::Optimization(opt) => {
384-
let enabled = match cgcx.remark {
385-
Passes::All => true,
386-
Passes::Some(ref v) => v.iter().any(|s| *s == opt.pass_name),
387-
};
388-
389-
if enabled {
390-
diag_handler.emit_note(FromLlvmOptimizationDiag {
391-
filename: &opt.filename,
392-
line: opt.line,
393-
column: opt.column,
394-
pass_name: &opt.pass_name,
395-
kind: match opt.kind {
396-
OptimizationDiagnosticKind::OptimizationRemark => "success",
397-
OptimizationDiagnosticKind::OptimizationMissed
398-
| OptimizationDiagnosticKind::OptimizationFailure => "missed",
399-
OptimizationDiagnosticKind::OptimizationAnalysis
400-
| OptimizationDiagnosticKind::OptimizationAnalysisFPCommute
401-
| OptimizationDiagnosticKind::OptimizationAnalysisAliasing => "analysis",
402-
OptimizationDiagnosticKind::OptimizationRemarkOther => "other",
403-
},
404-
message: &opt.message,
405-
});
406-
}
384+
diag_handler.emit_note(FromLlvmOptimizationDiag {
385+
filename: &opt.filename,
386+
line: opt.line,
387+
column: opt.column,
388+
pass_name: &opt.pass_name,
389+
kind: match opt.kind {
390+
OptimizationDiagnosticKind::OptimizationRemark => "success",
391+
OptimizationDiagnosticKind::OptimizationMissed
392+
| OptimizationDiagnosticKind::OptimizationFailure => "missed",
393+
OptimizationDiagnosticKind::OptimizationAnalysis
394+
| OptimizationDiagnosticKind::OptimizationAnalysisFPCommute
395+
| OptimizationDiagnosticKind::OptimizationAnalysisAliasing => "analysis",
396+
OptimizationDiagnosticKind::OptimizationRemarkOther => "other",
397+
},
398+
message: &opt.message,
399+
});
407400
}
408401
llvm::diagnostic::PGO(diagnostic_ref) | llvm::diagnostic::Linker(diagnostic_ref) => {
409402
let message = llvm::build_string(|s| {

Diff for: compiler/rustc_codegen_llvm/src/llvm/diagnostic.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use libc::c_uint;
99
use super::{DiagnosticInfo, SMDiagnostic};
1010
use rustc_span::InnerSpan;
1111

12-
#[derive(Copy, Clone)]
12+
#[derive(Copy, Clone, Debug)]
1313
pub enum OptimizationDiagnosticKind {
1414
OptimizationRemark,
1515
OptimizationMissed,

Diff for: compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp

+10-3
Original file line numberDiff line numberDiff line change
@@ -1892,12 +1892,19 @@ extern "C" void LLVMRustContextConfigureDiagnosticHandler(
18921892
LlvmRemarkStreamer(std::move(LlvmRemarkStreamer)) {}
18931893

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

0 commit comments

Comments
 (0)