[llvm] r218784 - LTO: Ignore disabled diagnostic remarks

Duncan P. N. Exon Smith dexonsmith at apple.com
Wed Oct 1 11:36:03 PDT 2014


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);
 }

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);





More information about the llvm-commits mailing list