Skip to content

Commit

Permalink
Rollup merge of #90833 - tmiasko:optimization-remarks, r=nikic
Browse files Browse the repository at this point in the history
Emit LLVM optimization remarks when enabled with `-Cremark`

The default diagnostic handler considers all remarks to be disabled by
default unless configured otherwise through LLVM internal flags:
`-pass-remarks`, `-pass-remarks-missed`, and `-pass-remarks-analysis`.
This behaviour makes `-Cremark` ineffective on its own.

Fix this by configuring a custom diagnostic handler that enables
optimization remarks based on the value of `-Cremark` option. With
`-Cremark=all` enabling all remarks.

Fixes #90924.

r? `@nikic`
  • Loading branch information
matthiaskrgr authored Nov 28, 2021
2 parents 9ef0bcf + 8fa4529 commit 67762ff
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 19 deletions.
39 changes: 29 additions & 10 deletions compiler/rustc_codegen_llvm/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,7 @@ pub(crate) fn save_temp_bitcode(
pub struct DiagnosticHandlers<'a> {
data: *mut (&'a CodegenContext<LlvmCodegenBackend>, &'a Handler),
llcx: &'a llvm::Context,
old_handler: Option<&'a llvm::DiagnosticHandler>,
}

impl<'a> DiagnosticHandlers<'a> {
Expand All @@ -267,12 +268,35 @@ impl<'a> DiagnosticHandlers<'a> {
handler: &'a Handler,
llcx: &'a llvm::Context,
) -> Self {
let remark_passes_all: bool;
let remark_passes: Vec<CString>;
match &cgcx.remark {
Passes::All => {
remark_passes_all = true;
remark_passes = Vec::new();
}
Passes::Some(passes) => {
remark_passes_all = false;
remark_passes =
passes.iter().map(|name| CString::new(name.as_str()).unwrap()).collect();
}
};
let remark_passes: Vec<*const c_char> =
remark_passes.iter().map(|name: &CString| name.as_ptr()).collect();
let data = Box::into_raw(Box::new((cgcx, handler)));
unsafe {
let old_handler = llvm::LLVMRustContextGetDiagnosticHandler(llcx);
llvm::LLVMRustContextConfigureDiagnosticHandler(
llcx,
diagnostic_handler,
data.cast(),
remark_passes_all,
remark_passes.as_ptr(),
remark_passes.len(),
);
llvm::LLVMRustSetInlineAsmDiagnosticHandler(llcx, inline_asm_handler, data.cast());
llvm::LLVMContextSetDiagnosticHandler(llcx, diagnostic_handler, data.cast());
DiagnosticHandlers { data, llcx, old_handler }
}
DiagnosticHandlers { data, llcx }
}
}

Expand All @@ -281,7 +305,7 @@ impl<'a> Drop for DiagnosticHandlers<'a> {
use std::ptr::null_mut;
unsafe {
llvm::LLVMRustSetInlineAsmDiagnosticHandler(self.llcx, inline_asm_handler, null_mut());
llvm::LLVMContextSetDiagnosticHandler(self.llcx, diagnostic_handler, null_mut());
llvm::LLVMRustContextSetDiagnosticHandler(self.llcx, self.old_handler);
drop(Box::from_raw(self.data));
}
}
Expand Down Expand Up @@ -337,13 +361,8 @@ unsafe extern "C" fn diagnostic_handler(info: &DiagnosticInfo, user: *mut c_void

if enabled {
diag_handler.note_without_error(&format!(
"optimization {} for {} at {}:{}:{}: {}",
opt.kind.describe(),
opt.pass_name,
opt.filename,
opt.line,
opt.column,
opt.message
"{}:{}:{}: {}: {}",
opt.filename, opt.line, opt.column, opt.pass_name, opt.message,
));
}
}
Expand Down
31 changes: 22 additions & 9 deletions compiler/rustc_codegen_llvm/src/llvm/ffi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -675,8 +675,12 @@ pub struct OperandBundleDef<'a>(InvariantOpaque<'a>);
#[repr(C)]
pub struct Linker<'a>(InvariantOpaque<'a>);

pub type DiagnosticHandler = unsafe extern "C" fn(&DiagnosticInfo, *mut c_void);
pub type InlineAsmDiagHandler = unsafe extern "C" fn(&SMDiagnostic, *const c_void, c_uint);
extern "C" {
pub type DiagnosticHandler;
}

pub type DiagnosticHandlerTy = unsafe extern "C" fn(&DiagnosticInfo, *mut c_void);
pub type InlineAsmDiagHandlerTy = unsafe extern "C" fn(&SMDiagnostic, *const c_void, c_uint);

pub mod coverageinfo {
use super::coverage_map;
Expand Down Expand Up @@ -2289,12 +2293,6 @@ extern "C" {
#[allow(improper_ctypes)]
pub fn LLVMRustWriteTwineToString(T: &Twine, s: &RustString);

pub fn LLVMContextSetDiagnosticHandler(
C: &Context,
Handler: DiagnosticHandler,
DiagnosticContext: *mut c_void,
);

#[allow(improper_ctypes)]
pub fn LLVMRustUnpackOptimizationDiagnostic(
DI: &'a DiagnosticInfo,
Expand Down Expand Up @@ -2324,7 +2322,7 @@ extern "C" {

pub fn LLVMRustSetInlineAsmDiagnosticHandler(
C: &Context,
H: InlineAsmDiagHandler,
H: InlineAsmDiagHandlerTy,
CX: *mut c_void,
);

Expand Down Expand Up @@ -2439,4 +2437,19 @@ extern "C" {
mod_id: *const c_char,
data: &ThinLTOData,
);

pub fn LLVMRustContextGetDiagnosticHandler(Context: &Context) -> Option<&DiagnosticHandler>;
pub fn LLVMRustContextSetDiagnosticHandler(
context: &Context,
diagnostic_handler: Option<&DiagnosticHandler>,
);
pub fn LLVMRustContextConfigureDiagnosticHandler(
context: &Context,
diagnostic_handler_callback: DiagnosticHandlerTy,
diagnostic_handler_context: *mut c_void,
remark_all_passes: bool,
remark_passes: *const *const c_char,
remark_passes_len: usize,
);

}
93 changes: 93 additions & 0 deletions compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "LLVMWrapper.h"
#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/DiagnosticHandler.h"
#include "llvm/IR/DiagnosticInfo.h"
#include "llvm/IR/DiagnosticPrinter.h"
#include "llvm/IR/GlobalVariable.h"
Expand Down Expand Up @@ -1177,10 +1178,13 @@ static LLVMRustDiagnosticKind toRust(DiagnosticKind Kind) {
case DK_SampleProfile:
return LLVMRustDiagnosticKind::SampleProfile;
case DK_OptimizationRemark:
case DK_MachineOptimizationRemark:
return LLVMRustDiagnosticKind::OptimizationRemark;
case DK_OptimizationRemarkMissed:
case DK_MachineOptimizationRemarkMissed:
return LLVMRustDiagnosticKind::OptimizationRemarkMissed;
case DK_OptimizationRemarkAnalysis:
case DK_MachineOptimizationRemarkAnalysis:
return LLVMRustDiagnosticKind::OptimizationRemarkAnalysis;
case DK_OptimizationRemarkAnalysisFPCommute:
return LLVMRustDiagnosticKind::OptimizationRemarkAnalysisFPCommute;
Expand Down Expand Up @@ -1783,3 +1787,92 @@ extern "C" LLVMRustResult LLVMRustWriteImportLibrary(
return LLVMRustResult::Success;
}
}

// Transfers ownership of DiagnosticHandler unique_ptr to the caller.
extern "C" DiagnosticHandler *
LLVMRustContextGetDiagnosticHandler(LLVMContextRef C) {
std::unique_ptr<DiagnosticHandler> DH = unwrap(C)->getDiagnosticHandler();
return DH.release();
}

// Sets unique_ptr to object of DiagnosticHandler to provide custom diagnostic
// handling. Ownership of the handler is moved to the LLVMContext.
extern "C" void LLVMRustContextSetDiagnosticHandler(LLVMContextRef C,
DiagnosticHandler *DH) {
unwrap(C)->setDiagnosticHandler(std::unique_ptr<DiagnosticHandler>(DH));
}

using LLVMDiagnosticHandlerTy = DiagnosticHandler::DiagnosticHandlerTy;

// Configures a diagnostic handler that invokes provided callback when a
// backend needs to emit a diagnostic.
//
// When RemarkAllPasses is true, remarks are enabled for all passes. Otherwise
// the RemarkPasses array specifies individual passes for which remarks will be
// enabled.
extern "C" void LLVMRustContextConfigureDiagnosticHandler(
LLVMContextRef C, LLVMDiagnosticHandlerTy DiagnosticHandlerCallback,
void *DiagnosticHandlerContext, bool RemarkAllPasses,
const char * const * RemarkPasses, size_t RemarkPassesLen) {

class RustDiagnosticHandler final : public DiagnosticHandler {
public:
RustDiagnosticHandler(LLVMDiagnosticHandlerTy DiagnosticHandlerCallback,
void *DiagnosticHandlerContext,
bool RemarkAllPasses,
std::vector<std::string> RemarkPasses)
: DiagnosticHandlerCallback(DiagnosticHandlerCallback),
DiagnosticHandlerContext(DiagnosticHandlerContext),
RemarkAllPasses(RemarkAllPasses),
RemarkPasses(RemarkPasses) {}

virtual bool handleDiagnostics(const DiagnosticInfo &DI) override {
if (DiagnosticHandlerCallback) {
DiagnosticHandlerCallback(DI, DiagnosticHandlerContext);
return true;
}
return false;
}

bool isAnalysisRemarkEnabled(StringRef PassName) const override {
return isRemarkEnabled(PassName);
}

bool isMissedOptRemarkEnabled(StringRef PassName) const override {
return isRemarkEnabled(PassName);
}

bool isPassedOptRemarkEnabled(StringRef PassName) const override {
return isRemarkEnabled(PassName);
}

bool isAnyRemarkEnabled() const override {
return RemarkAllPasses || !RemarkPasses.empty();
}

private:
bool isRemarkEnabled(StringRef PassName) const {
if (RemarkAllPasses)
return true;

for (auto &Pass : RemarkPasses)
if (Pass == PassName)
return true;

return false;
}

LLVMDiagnosticHandlerTy DiagnosticHandlerCallback = nullptr;
void *DiagnosticHandlerContext = nullptr;

bool RemarkAllPasses = false;
std::vector<std::string> RemarkPasses;
};

std::vector<std::string> Passes;
for (size_t I = 0; I != RemarkPassesLen; ++I)
Passes.push_back(RemarkPasses[I]);

unwrap(C)->setDiagnosticHandler(std::make_unique<RustDiagnosticHandler>(
DiagnosticHandlerCallback, DiagnosticHandlerContext, RemarkAllPasses, Passes));
}
19 changes: 19 additions & 0 deletions src/test/ui/optimization-remark.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// build-pass
// ignore-pass
// no-system-llvm
// revisions: all inline
// compile-flags: --crate-type=lib -Cdebuginfo=1 -Copt-level=2
// [all] compile-flags: -Cremark=all
// [inline] compile-flags: -Cremark=inline
// error-pattern: inline: f not inlined into g
// dont-check-compiler-stderr

#[no_mangle]
#[inline(never)]
pub fn f() {
}

#[no_mangle]
pub fn g() {
f();
}

0 comments on commit 67762ff

Please sign in to comment.