[llvm] r218784 - LTO: Ignore disabled diagnostic remarks
Justin Bogner
mail at justinbogner.com
Wed Oct 1 12:07:59 PDT 2014
"Duncan P. N. Exon Smith" <dexonsmith at apple.com> writes:
> Author: dexonsmith
> Date: Wed Oct 1 13:36:03 2014
> New Revision: 218784
>
> URL: http://llvm.org/viewvc/llvm-project?rev=218784&view=rev
> Log:
> LTO: Ignore disabled diagnostic remarks
>
> r206400 and r209442 added remarks that are disabled by default.
> However, if a diagnostic handler is registered, the remarks are sent
> unfiltered to the handler. This is the right behaviour for clang, since
> it has its own filters.
>
> However, the diagnostic handler exposed in the LTO API receives only the
> severity and message. It doesn't have the information to filter by pass
> name. For LTO, disabled remarks should be filtered by the producer.
>
> I've changed `LLVMContext::setDiagnosticHandler()` to take a `bool`
> argument indicating whether to respect the built-in filters. This
> defaults to `false`, so other consumers don't have a behaviour change,
> but `LTOCodeGenerator::setDiagnosticHandler()` sets it to `true`.
>
> To make this behaviour testable, I added a `-use-diagnostic-handler`
> command-line option to `llvm-lto`.
>
> This fixes PR21108.
>
> Added:
> llvm/trunk/test/LTO/diagnostic-handler-remarks.ll
> Modified:
> llvm/trunk/include/llvm/IR/LLVMContext.h
> llvm/trunk/lib/IR/LLVMContext.cpp
> llvm/trunk/lib/IR/LLVMContextImpl.cpp
> llvm/trunk/lib/IR/LLVMContextImpl.h
> llvm/trunk/lib/LTO/LTOCodeGenerator.cpp
> llvm/trunk/tools/llvm-lto/llvm-lto.cpp
>
> Modified: llvm/trunk/include/llvm/IR/LLVMContext.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/LLVMContext.h?rev=218784&r1=218783&r2=218784&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/IR/LLVMContext.h (original)
> +++ llvm/trunk/include/llvm/IR/LLVMContext.h Wed Oct 1 13:36:03 2014
> @@ -99,12 +99,14 @@ public:
> /// setDiagnosticHandler - This method sets a handler that is invoked
> /// when the backend needs to report anything to the user. The first
> /// argument is a function pointer and the second is a context pointer that
> - /// gets passed into the DiagHandler.
> + /// gets passed into the DiagHandler. The third argument should be set to
> + /// true if the handler only expects enabled diagnostics.
> ///
> /// LLVMContext doesn't take ownership or interpret either of these
> /// pointers.
> void setDiagnosticHandler(DiagnosticHandlerTy DiagHandler,
> - void *DiagContext = nullptr);
> + void *DiagContext = nullptr,
> + bool RespectFilters = false);
>
> /// getDiagnosticHandler - Return the diagnostic handler set by
> /// setDiagnosticHandler.
>
> Modified: llvm/trunk/lib/IR/LLVMContext.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContext.cpp?rev=218784&r1=218783&r2=218784&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/LLVMContext.cpp (original)
> +++ llvm/trunk/lib/IR/LLVMContext.cpp Wed Oct 1 13:36:03 2014
> @@ -112,9 +112,11 @@ void *LLVMContext::getInlineAsmDiagnosti
> }
>
> void LLVMContext::setDiagnosticHandler(DiagnosticHandlerTy DiagnosticHandler,
> - void *DiagnosticContext) {
> + void *DiagnosticContext,
> + bool RespectFilters) {
> pImpl->DiagnosticHandler = DiagnosticHandler;
> pImpl->DiagnosticContext = DiagnosticContext;
> + pImpl->RespectDiagnosticFilters = RespectFilters;
> }
>
> LLVMContext::DiagnosticHandlerTy LLVMContext::getDiagnosticHandler() const {
> @@ -145,13 +147,7 @@ void LLVMContext::emitError(const Instru
> diagnose(DiagnosticInfoInlineAsm(*I, ErrorStr));
> }
>
> -void LLVMContext::diagnose(const DiagnosticInfo &DI) {
> - // If there is a report handler, use it.
> - if (pImpl->DiagnosticHandler) {
> - pImpl->DiagnosticHandler(DI, pImpl->DiagnosticContext);
> - return;
> - }
> -
> +static bool isDiagnosticEnabled(const DiagnosticInfo &DI) {
> // Optimization remarks are selective. They need to check whether the regexp
> // pattern, passed via one of the -pass-remarks* flags, matches the name of
> // the pass that is emitting the diagnostic. If there is no match, ignore the
> @@ -159,19 +155,32 @@ void LLVMContext::diagnose(const Diagnos
> switch (DI.getKind()) {
> case llvm::DK_OptimizationRemark:
> if (!cast<DiagnosticInfoOptimizationRemark>(DI).isEnabled())
> - return;
> + return false;
> break;
> case llvm::DK_OptimizationRemarkMissed:
> if (!cast<DiagnosticInfoOptimizationRemarkMissed>(DI).isEnabled())
> - return;
> + return false;
> break;
> case llvm::DK_OptimizationRemarkAnalysis:
> if (!cast<DiagnosticInfoOptimizationRemarkAnalysis>(DI).isEnabled())
> - return;
> + return false;
> break;
> default:
> break;
> }
> + return true;
> +}
> +
> +void LLVMContext::diagnose(const DiagnosticInfo &DI) {
> + // If there is a report handler, use it.
> + if (pImpl->DiagnosticHandler) {
> + if (!pImpl->RespectDiagnosticFilters || isDiagnosticEnabled(DI))
> + pImpl->DiagnosticHandler(DI, pImpl->DiagnosticContext);
> + return;
> + }
> +
> + if (!isDiagnosticEnabled(DI))
> + return;
>
> // Otherwise, print the message with a prefix based on the severity.
> std::string MsgStorage;
>
> Modified: llvm/trunk/lib/IR/LLVMContextImpl.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.cpp?rev=218784&r1=218783&r2=218784&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/LLVMContextImpl.cpp (original)
> +++ llvm/trunk/lib/IR/LLVMContextImpl.cpp Wed Oct 1 13:36:03 2014
> @@ -40,6 +40,7 @@ LLVMContextImpl::LLVMContextImpl(LLVMCon
> InlineAsmDiagContext = nullptr;
> DiagnosticHandler = nullptr;
> DiagnosticContext = nullptr;
> + RespectDiagnosticFilters = false;
> YieldCallback = nullptr;
> YieldOpaqueHandle = nullptr;
> NamedStructTypesUniqueID = 0;
>
> Modified: llvm/trunk/lib/IR/LLVMContextImpl.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.h?rev=218784&r1=218783&r2=218784&view=diff
> ==============================================================================
> --- llvm/trunk/lib/IR/LLVMContextImpl.h (original)
> +++ llvm/trunk/lib/IR/LLVMContextImpl.h Wed Oct 1 13:36:03 2014
> @@ -244,6 +244,7 @@ public:
>
> LLVMContext::DiagnosticHandlerTy DiagnosticHandler;
> void *DiagnosticContext;
> + bool RespectDiagnosticFilters;
>
> LLVMContext::YieldCallbackTy YieldCallback;
> void *YieldOpaqueHandle;
>
> Modified: llvm/trunk/lib/LTO/LTOCodeGenerator.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTOCodeGenerator.cpp?rev=218784&r1=218783&r2=218784&view=diff
> ==============================================================================
> --- llvm/trunk/lib/LTO/LTOCodeGenerator.cpp (original)
> +++ llvm/trunk/lib/LTO/LTOCodeGenerator.cpp Wed Oct 1 13:36:03 2014
> @@ -558,5 +558,5 @@ LTOCodeGenerator::setDiagnosticHandler(l
> return Context.setDiagnosticHandler(nullptr, nullptr);
> // Register the LTOCodeGenerator stub in the LLVMContext to forward the
> // diagnostic to the external DiagHandler.
> - Context.setDiagnosticHandler(LTOCodeGenerator::DiagnosticHandler, this);
> + Context.setDiagnosticHandler(LTOCodeGenerator::DiagnosticHandler, this, true);
This reads a little bit nicer if you explain what the boolean argument
is here, like /*RespectFilters=*/true
> }
>
> Added: llvm/trunk/test/LTO/diagnostic-handler-remarks.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/LTO/diagnostic-handler-remarks.ll?rev=218784&view=auto
> ==============================================================================
> --- llvm/trunk/test/LTO/diagnostic-handler-remarks.ll (added)
> +++ llvm/trunk/test/LTO/diagnostic-handler-remarks.ll Wed Oct 1 13:36:03 2014
> @@ -0,0 +1,38 @@
> +; RUN: llvm-as < %s >%t.bc
> +; PR21108: Diagnostic handlers get pass remarks, even if they're not enabled.
> +
> +; Confirm that there are -pass-remarks.
> +; RUN: llvm-lto -pass-remarks=inline \
> +; RUN: -exported-symbol _main -o %t.o %t.bc 2>&1 | \
> +; RUN: FileCheck %s -allow-empty -check-prefix=REMARKS
> +; RUN: llvm-nm %t.o | FileCheck %s -check-prefix NM
> +
> +; RUN: llvm-lto -pass-remarks=inline -use-diagnostic-handler \
> +; RUN: -exported-symbol _main -o %t.o %t.bc 2>&1 | \
> +; RUN: FileCheck %s -allow-empty -check-prefix=REMARKS
> +; RUN: llvm-nm %t.o | FileCheck %s -check-prefix NM
> +
> +; Confirm that -pass-remarks are not printed by default.
> +; RUN: llvm-lto \
> +; RUN: -exported-symbol _main -o %t.o %t.bc 2>&1 | \
> +; RUN: FileCheck %s -allow-empty
> +; RUN: llvm-nm %t.o | FileCheck %s -check-prefix NM
> +
> +; RUN: llvm-lto -use-diagnostic-handler \
> +; RUN: -exported-symbol _main -o %t.o %t.bc 2>&1 | \
> +; RUN: FileCheck %s -allow-empty
> +; RUN: llvm-nm %t.o | FileCheck %s -check-prefix NM
> +
> +; REMARKS: remark:
> +; CHECK-NOT: remark:
> +; NM-NOT: foo
> +; NM: main
> +
> +define i32 @foo() {
> + ret i32 7
> +}
> +
> +define i32 @main() {
> + %i = call i32 @foo()
> + ret i32 %i
> +}
>
> Modified: llvm/trunk/tools/llvm-lto/llvm-lto.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-lto/llvm-lto.cpp?rev=218784&r1=218783&r2=218784&view=diff
> ==============================================================================
> --- llvm/trunk/tools/llvm-lto/llvm-lto.cpp (original)
> +++ llvm/trunk/tools/llvm-lto/llvm-lto.cpp Wed Oct 1 13:36:03 2014
> @@ -38,6 +38,10 @@ static cl::opt<bool>
> DisableGVNLoadPRE("disable-gvn-loadpre", cl::init(false),
> cl::desc("Do not run the GVN load PRE pass"));
>
> +static cl::opt<bool>
> +UseDiagnosticHandler("use-diagnostic-handler", cl::init(false),
> + cl::desc("Use a diagnostic handler to test the handler interface"));
> +
> static cl::list<std::string>
> InputFilenames(cl::Positional, cl::OneOrMore,
> cl::desc("<input bitcode files>"));
> @@ -63,6 +67,25 @@ struct ModuleInfo {
> };
> }
>
> +void handleDiagnostics(lto_codegen_diagnostic_severity_t Severity,
> + const char *Msg, void *) {
> + switch (Severity) {
> + case LTO_DS_NOTE:
> + errs() << "note: ";
> + break;
> + case LTO_DS_REMARK:
> + errs() << "remark: ";
> + break;
> + case LTO_DS_ERROR:
> + errs() << "error: ";
> + break;
> + case LTO_DS_WARNING:
> + errs() << "warning: ";
> + break;
> + }
> + errs() << Msg << "\n";
> +}
> +
> int main(int argc, char **argv) {
> // Print a stack trace if we signal out.
> sys::PrintStackTraceOnErrorSignal();
> @@ -84,6 +107,9 @@ int main(int argc, char **argv) {
>
> LTOCodeGenerator CodeGen;
>
> + if (UseDiagnosticHandler)
> + CodeGen.setDiagnosticHandler(handleDiagnostics, nullptr);
> +
> switch (RelocModel) {
> case Reloc::Static:
> CodeGen.setCodePICModel(LTO_CODEGEN_PIC_MODEL_STATIC);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list