This patch fixes https://bugs.llvm.org/show_bug.cgi?id=32352

It enables OptimizationRemarkEmitter::allowExtraAnalysis and MachineOptimizationRemarkEmitter::allowExtraAnalysis to return true not only for -fsave-optimization-record but when specific remarks are requested with
command line options.
The diagnostic handler used to be callback now this patch adds a class
DiagnosticHandler. It has virtual method to provide custom diagnostic handler
and methods to control which particular remarks are enabled. 
However LLVM-C API users can still provide callback function for diagnostic handler.

llvm-svn: 313382

include/llvm/Analysis/OptimizationDiagnosticInfo.h

@@ -78,10 +78,9 @@ public:
   /// use the extra analysis (1) to filter trivial false positives or (2) to
   /// provide more context so that non-trivial false positives can be quickly
   /// detected by the user.
-  bool allowExtraAnalysis() const {
-    // For now, only allow this with -fsave-optimization-record since the -Rpass
-    // options are handled in the front-end.
-    return F->getContext().getDiagnosticsOutputFile();
+  bool allowExtraAnalysis(StringRef PassName) const {
+    return (F->getContext().getDiagnosticsOutputFile() ||
+            F->getContext().getDiagHandlerPtr()->isAnyRemarkEnabled(PassName));
   }
 
 private:

include/llvm/CodeGen/MachineOptimizationRemarkEmitter.h

@@ -73,7 +73,9 @@ public:
 
   /// \see DiagnosticInfoOptimizationBase::isEnabled.
   bool isEnabled() const override {
-    return OptimizationRemark::isEnabled(getPassName());
+    const Function &Fn = getFunction();
+    LLVMContext &Ctx = Fn.getContext();
+    return Ctx.getDiagHandlerPtr()->isPassedOptRemarkEnabled(getPassName());
   }
 };
 
@@ -97,7 +99,9 @@ public:
 
   /// \see DiagnosticInfoOptimizationBase::isEnabled.
   bool isEnabled() const override {
-    return OptimizationRemarkMissed::isEnabled(getPassName());
+    const Function &Fn = getFunction();
+    LLVMContext &Ctx = Fn.getContext();
+    return Ctx.getDiagHandlerPtr()->isMissedOptRemarkEnabled(getPassName());
   }
 };
 
@@ -121,7 +125,9 @@ public:
 
   /// \see DiagnosticInfoOptimizationBase::isEnabled.
   bool isEnabled() const override {
-    return OptimizationRemarkAnalysis::isEnabled(getPassName());
+    const Function &Fn = getFunction();
+    LLVMContext &Ctx = Fn.getContext();
+    return Ctx.getDiagHandlerPtr()->isAnalysisRemarkEnabled(getPassName());
   }
 };
 
@@ -152,10 +158,10 @@ public:
   /// that are normally too noisy.  In this mode, we can use the extra analysis
   /// (1) to filter trivial false positives or (2) to provide more context so
   /// that non-trivial false positives can be quickly detected by the user.
-  bool allowExtraAnalysis() const {
-    // For now, only allow this with -fsave-optimization-record since the -Rpass
-    // options are handled in the front-end.
-    return MF.getFunction()->getContext().getDiagnosticsOutputFile();
+  bool allowExtraAnalysis(StringRef PassName) const {
+    return (MF.getFunction()->getContext().getDiagnosticsOutputFile() ||
+            MF.getFunction()->getContext()
+            .getDiagHandlerPtr()->isAnyRemarkEnabled(PassName));
   }
 
 private:

include/llvm/IR/DiagnosticHandler.h

@@ -0,0 +1,67 @@
+//===- DiagnosticHandler.h - DiagnosticHandler class for LLVM -*- C++ ---*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+// Base DiagnosticHandler class declaration. Derive from this class to provide
+// custom diagnostic reporting.
+//===----------------------------------------------------------------------===//
+
+#include "llvm/ADT/StringRef.h"
+
+namespace llvm {
+class DiagnosticInfo;
+
+/// \brief This is the base class for diagnostic handling in LLVM.
+/// The handleDiagnostics method must be overriden by the subclasses to handle
+/// diagnostic. The *RemarkEnabled methods can be overriden to control
+/// which remarks are enabled.
+struct DiagnosticHandler {
+  void *DiagnosticContext = nullptr;
+  DiagnosticHandler(void *DiagContext = nullptr)
+      : DiagnosticContext(DiagContext) {}
+  virtual ~DiagnosticHandler() = default;
+
+  using DiagnosticHandlerTy = void (*)(const DiagnosticInfo &DI, void *Context);
+
+  /// DiagHandlerCallback is settable from the C API and base implementation
+  /// of DiagnosticHandler will call it from handleDiagnostics(). Any derived
+  /// class of DiagnosticHandler should not use callback but
+  /// implement handleDiagnostics().
+  DiagnosticHandlerTy DiagHandlerCallback = nullptr;
+
+  /// Override handleDiagnostics to provide custom implementation.
+  /// Return true if it handles diagnostics reporting properly otherwise
+  /// return false to make LLVMContext::diagnose() to print the message
+  /// with a prefix based on the severity.
+  virtual bool handleDiagnostics(const DiagnosticInfo &DI) {
+    if (DiagHandlerCallback) {
+      DiagHandlerCallback(DI, DiagnosticContext);
+      return true;
+    }
+    return false;
+  }
+
+  /// Return true if analysis remarks are enabled, override
+  /// to provide different implementation.
+  virtual bool isAnalysisRemarkEnabled(StringRef PassName) const;
+
+  /// Return true if missed optimization remarks are enabled, override
+  /// to provide different implementation.
+  virtual bool isMissedOptRemarkEnabled(StringRef PassName) const;
+
+  /// Return true if passed optimization remarks are enabled, override
+  /// to provide different implementation.
+  virtual bool isPassedOptRemarkEnabled(StringRef PassName) const;
+
+  /// Return true if any type of remarks are enabled.
+  bool isAnyRemarkEnabled(StringRef PassName) const {
+    return (isMissedOptRemarkEnabled(PassName) ||
+            isPassedOptRemarkEnabled(PassName) ||
+            isAnalysisRemarkEnabled(PassName));
+  }
+};
+}

include/llvm/IR/DiagnosticInfo.h

@@ -604,10 +604,8 @@ public:
     return DI->getKind() == DK_OptimizationRemark;
   }
 
-  static bool isEnabled(StringRef PassName);
-
   /// \see DiagnosticInfoOptimizationBase::isEnabled.
-  bool isEnabled() const override { return isEnabled(getPassName()); }
+  bool isEnabled() const override;
 
 private:
   /// This is deprecated now and only used by the function API below.
@@ -647,10 +645,8 @@ public:
     return DI->getKind() == DK_OptimizationRemarkMissed;
   }
 
-  static bool isEnabled(StringRef PassName);
-
   /// \see DiagnosticInfoOptimizationBase::isEnabled.
-  bool isEnabled() const override { return isEnabled(getPassName()); }
+  bool isEnabled() const override;
 
 private:
   /// This is deprecated now and only used by the function API below.
@@ -701,12 +697,8 @@ public:
     return DI->getKind() == DK_OptimizationRemarkAnalysis;
   }
 
-  static bool isEnabled(StringRef PassName);
-
   /// \see DiagnosticInfoOptimizationBase::isEnabled.
-  bool isEnabled() const override {
-    return shouldAlwaysPrint() || isEnabled(getPassName());
-  }
+  bool isEnabled() const override;
 
   static const char *AlwaysPrint;
 

include/llvm/IR/LLVMContext.h

@@ -16,6 +16,7 @@
 #define LLVM_IR_LLVMCONTEXT_H
 
 #include "llvm-c/Types.h"
+#include "llvm/IR/DiagnosticHandler.h"
 #include "llvm/Support/CBindingWrapping.h"
 #include "llvm/Support/Options.h"
 #include <cstdint>
@@ -167,11 +168,6 @@ public:
   using InlineAsmDiagHandlerTy = void (*)(const SMDiagnostic&, void *Context,
                                           unsigned LocCookie);
 
-  /// Defines the type of a diagnostic handler.
-  /// \see LLVMContext::setDiagnosticHandler.
-  /// \see LLVMContext::diagnose.
-  using DiagnosticHandlerTy = void (*)(const DiagnosticInfo &DI, void *Context);
-
   /// Defines the type of a yield callback.
   /// \see LLVMContext::setYieldCallback.
   using YieldCallbackTy = void (*)(LLVMContext *Context, void *OpaqueHandle);
@@ -194,26 +190,43 @@ public:
   /// setInlineAsmDiagnosticHandler.
   void *getInlineAsmDiagnosticContext() const;
 
-  /// 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.  The third argument should be set to
+  /// setDiagnosticHandlerCallBack - This method sets a handler call back
+  /// 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.  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 setDiagnosticHandlerCallBack(
+      DiagnosticHandler::DiagnosticHandlerTy DiagHandler,
+      void *DiagContext = nullptr, bool RespectFilters = false);
+
+  /// setDiagnosticHandler - This method sets unique_ptr to object of DiagnosticHandler
+  /// to provide custom diagnostic handling. The first argument is unique_ptr of object
+  /// of type DiagnosticHandler or a derived of that.   The third argument should be
+  /// set to true if the handler only expects enabled diagnostics.
+  ///
+  /// Ownership of this pointer is moved to LLVMContextImpl.
+  void setDiagnosticHandler(std::unique_ptr<DiagnosticHandler> &&DH,
                             bool RespectFilters = false);
 
-  /// getDiagnosticHandler - Return the diagnostic handler set by
-  /// setDiagnosticHandler.
-  DiagnosticHandlerTy getDiagnosticHandler() const;
+  /// getDiagnosticHandlerCallBack - Return the diagnostic handler call back set by
+  /// setDiagnosticHandlerCallBack.
+  DiagnosticHandler::DiagnosticHandlerTy getDiagnosticHandlerCallBack() const;
 
   /// getDiagnosticContext - Return the diagnostic context set by
   /// setDiagnosticContext.
   void *getDiagnosticContext() const;
 
+  /// getDiagHandlerPtr - Returns const raw pointer of DiagnosticHandler set by
+  /// setDiagnosticHandler.
+  const DiagnosticHandler *getDiagHandlerPtr() const;
+
+  /// getDiagnosticHandler - transfers owenership of DiagnosticHandler unique_ptr
+  /// to caller.
+  std::unique_ptr<DiagnosticHandler> getDiagnosticHandler();
+
   /// \brief Return if a code hotness metric should be included in optimization
   /// diagnostics.
   bool getDiagnosticsHotnessRequested() const;

include/llvm/LTO/Config.h

@@ -171,20 +171,27 @@ struct Config {
                      bool UseInputModulePath = false);
 };
 
+struct LTOLLVMDiagnosticHandler : public DiagnosticHandler {
+  DiagnosticHandlerFunction *Fn;
+  LTOLLVMDiagnosticHandler(DiagnosticHandlerFunction *DiagHandlerFn)
+      : Fn(DiagHandlerFn) {}
+  bool handleDiagnostics(const DiagnosticInfo &DI) override {
+    (*Fn)(DI);
+    return true;
+  }
+};
 /// A derived class of LLVMContext that initializes itself according to a given
 /// Config object. The purpose of this class is to tie ownership of the
 /// diagnostic handler to the context, as opposed to the Config object (which
 /// may be ephemeral).
+// FIXME: This should not be required as diagnostic handler is not callback.
 struct LTOLLVMContext : LLVMContext {
-  static void funcDiagHandler(const DiagnosticInfo &DI, void *Context) {
-    auto *Fn = static_cast<DiagnosticHandlerFunction *>(Context);
-    (*Fn)(DI);
-  }
 
   LTOLLVMContext(const Config &C) : DiagHandler(C.DiagHandler) {
     setDiscardValueNames(C.ShouldDiscardValueNames);
     enableDebugTypeODRUniquing();
-    setDiagnosticHandler(funcDiagHandler, &DiagHandler, true);
+    setDiagnosticHandler(
+        llvm::make_unique<LTOLLVMDiagnosticHandler>(&DiagHandler), true);
   }
   DiagnosticHandlerFunction DiagHandler;
 };

include/llvm/LTO/legacy/LTOCodeGenerator.h

@@ -184,6 +184,7 @@ struct LTOCodeGenerator {
   LLVMContext &getContext() { return Context; }
 
   void resetMergedModule() { MergedModule.reset(); }
+  void DiagnosticHandler(const DiagnosticInfo &DI);
 
 private:
   void initializeLTOPasses();
@@ -204,10 +205,6 @@ private:
   bool determineTarget();
   std::unique_ptr<TargetMachine> createTargetMachine();
 
-  static void DiagnosticHandler(const DiagnosticInfo &DI, void *Context);
-
-  void DiagnosticHandler2(const DiagnosticInfo &DI);
-
   void emitError(const std::string &ErrMsg);
   void emitWarning(const std::string &ErrMsg);
 

lib/IR/CMakeLists.txt

@@ -17,6 +17,7 @@ add_llvm_library(LLVMCore
   DebugInfo.cpp
   DebugInfoMetadata.cpp
   DebugLoc.cpp
+  DiagnosticHandler.cpp
   DiagnosticInfo.cpp
   DiagnosticPrinter.cpp
   Dominators.cpp

lib/IR/Core.cpp

@@ -85,15 +85,15 @@ LLVMContextRef LLVMGetGlobalContext() { return wrap(&*GlobalContext); }
 void LLVMContextSetDiagnosticHandler(LLVMContextRef C,
                                      LLVMDiagnosticHandler Handler,
                                      void *DiagnosticContext) {
-  unwrap(C)->setDiagnosticHandler(
-      LLVM_EXTENSION reinterpret_cast<LLVMContext::DiagnosticHandlerTy>(
+  unwrap(C)->setDiagnosticHandlerCallBack(
+      LLVM_EXTENSION reinterpret_cast<DiagnosticHandler::DiagnosticHandlerTy>(
           Handler),
       DiagnosticContext);
 }
 
 LLVMDiagnosticHandler LLVMContextGetDiagnosticHandler(LLVMContextRef C) {
   return LLVM_EXTENSION reinterpret_cast<LLVMDiagnosticHandler>(
-      unwrap(C)->getDiagnosticHandler());
+      unwrap(C)->getDiagnosticHandlerCallBack());
 }
 
 void *LLVMContextGetDiagnosticContext(LLVMContextRef C) {

lib/IR/DiagnosticHandler.cpp

@@ -0,0 +1,86 @@
+//===- DiagnosticHandler.h - DiagnosticHandler class for LLVM -------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//
+//===----------------------------------------------------------------------===//
+#include "llvm/IR/DiagnosticHandler.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/RegEx.h"
+
+using namespace llvm;
+
+namespace {
+
+/// \brief Regular expression corresponding to the value given in one of the
+/// -pass-remarks* command line flags. Passes whose name matches this regexp
+/// will emit a diagnostic when calling the associated diagnostic function
+/// (emitOptimizationRemark, emitOptimizationRemarkMissed or
+/// emitOptimizationRemarkAnalysis).
+struct PassRemarksOpt {
+  std::shared_ptr<Regex> Pattern;
+
+  void operator=(const std::string &Val) {
+    // Create a regexp object to match pass names for emitOptimizationRemark.
+    if (!Val.empty()) {
+      Pattern = std::make_shared<Regex>(Val);
+      std::string RegexError;
+      if (!Pattern->isValid(RegexError))
+        report_fatal_error("Invalid regular expression '" + Val +
+                               "' in -pass-remarks: " + RegexError,
+                           false);
+    }
+  }
+};
+
+static PassRemarksOpt PassRemarksPassedOptLoc;
+static PassRemarksOpt PassRemarksMissedOptLoc;
+static PassRemarksOpt PassRemarksAnalysisOptLoc;
+
+// -pass-remarks
+//    Command line flag to enable emitOptimizationRemark()
+static cl::opt<PassRemarksOpt, true, cl::parser<std::string>> PassRemarks(
+    "pass-remarks", cl::value_desc("pattern"),
+    cl::desc("Enable optimization remarks from passes whose name match "
+             "the given regular expression"),
+    cl::Hidden, cl::location(PassRemarksPassedOptLoc), cl::ValueRequired,
+    cl::ZeroOrMore);
+
+// -pass-remarks-missed
+//    Command line flag to enable emitOptimizationRemarkMissed()
+static cl::opt<PassRemarksOpt, true, cl::parser<std::string>> PassRemarksMissed(
+    "pass-remarks-missed", cl::value_desc("pattern"),
+    cl::desc("Enable missed optimization remarks from passes whose name match "
+             "the given regular expression"),
+    cl::Hidden, cl::location(PassRemarksMissedOptLoc), cl::ValueRequired,
+    cl::ZeroOrMore);
+
+// -pass-remarks-analysis
+//    Command line flag to enable emitOptimizationRemarkAnalysis()
+static cl::opt<PassRemarksOpt, true, cl::parser<std::string>>
+    PassRemarksAnalysis(
+        "pass-remarks-analysis", cl::value_desc("pattern"),
+        cl::desc(
+            "Enable optimization analysis remarks from passes whose name match "
+            "the given regular expression"),
+        cl::Hidden, cl::location(PassRemarksAnalysisOptLoc), cl::ValueRequired,
+        cl::ZeroOrMore);
+}
+
+bool DiagnosticHandler::isAnalysisRemarkEnabled(StringRef PassName) const {
+  return (PassRemarksAnalysisOptLoc.Pattern &&
+          PassRemarksAnalysisOptLoc.Pattern->match(PassName));
+}
+bool DiagnosticHandler::isMissedOptRemarkEnabled(StringRef PassName) const {
+  return (PassRemarksMissedOptLoc.Pattern &&
+          PassRemarksMissedOptLoc.Pattern->match(PassName));
+}
+bool DiagnosticHandler::isPassedOptRemarkEnabled(StringRef PassName) const {
+  return (PassRemarksPassedOptLoc.Pattern &&
+          PassRemarksPassedOptLoc.Pattern->match(PassName));
+}

lib/IR/DiagnosticInfo.cpp

@@ -13,6 +13,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/IR/DiagnosticInfo.h"
+#include "LLVMContextImpl.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/iterator_range.h"
@@ -41,61 +42,6 @@
 
 using namespace llvm;
 
-namespace {
-
-/// \brief Regular expression corresponding to the value given in one of the
-/// -pass-remarks* command line flags. Passes whose name matches this regexp
-/// will emit a diagnostic via ORE->emit(...);
-struct PassRemarksOpt {
-  std::shared_ptr<Regex> Pattern;
-
-  void operator=(const std::string &Val) {
-    if (!Val.empty()) {
-      Pattern = std::make_shared<Regex>(Val);
-      std::string RegexError;
-      if (!Pattern->isValid(RegexError))
-        report_fatal_error("Invalid regular expression '" + Val +
-                               "' in -pass-remarks: " + RegexError,
-                           false);
-    }
-  }
-};
-
-} // end anonymous namespace
-
-static PassRemarksOpt PassRemarksOptLoc;
-static PassRemarksOpt PassRemarksMissedOptLoc;
-static PassRemarksOpt PassRemarksAnalysisOptLoc;
-
-// -pass-remarks
-//    Command line flag to enable optimization remarks
-static cl::opt<PassRemarksOpt, true, cl::parser<std::string>>
-PassRemarks("pass-remarks", cl::value_desc("pattern"),
-            cl::desc("Enable optimization remarks from passes whose name match "
-                     "the given regular expression"),
-            cl::Hidden, cl::location(PassRemarksOptLoc), cl::ValueRequired,
-            cl::ZeroOrMore);
-
-// -pass-remarks-missed
-//    Command line flag to enable missed optimization remarks
-static cl::opt<PassRemarksOpt, true, cl::parser<std::string>> PassRemarksMissed(
-    "pass-remarks-missed", cl::value_desc("pattern"),
-    cl::desc("Enable missed optimization remarks from passes whose name match "
-             "the given regular expression"),
-    cl::Hidden, cl::location(PassRemarksMissedOptLoc), cl::ValueRequired,
-    cl::ZeroOrMore);
-
-// -pass-remarks-analysis
-//    Command line flag to enable optimization analysis remarks
-static cl::opt<PassRemarksOpt, true, cl::parser<std::string>>
-PassRemarksAnalysis(
-    "pass-remarks-analysis", cl::value_desc("pattern"),
-    cl::desc(
-        "Enable optimization analysis remarks from passes whose name match "
-        "the given regular expression"),
-    cl::Hidden, cl::location(PassRemarksAnalysisOptLoc), cl::ValueRequired,
-    cl::ZeroOrMore);
-
 int llvm::getNextAvailablePluginDiagnosticKind() {
   static std::atomic<int> PluginKindID(DK_FirstPluginKind);
   return ++PluginKindID;
@@ -283,9 +229,10 @@ OptimizationRemark::OptimizationRemark(const char *PassName,
                                    RemarkName, *Func, Func->getSubprogram(),
                                    &getFirstFunctionBlock(Func)) {}
 
-bool OptimizationRemark::isEnabled(StringRef PassName) {
-  return PassRemarksOptLoc.Pattern &&
-         PassRemarksOptLoc.Pattern->match(PassName);
+bool OptimizationRemark::isEnabled() const {
+  const Function &Fn = getFunction();
+  LLVMContext &Ctx = Fn.getContext();
+  return Ctx.getDiagHandlerPtr()->isPassedOptRemarkEnabled(getPassName());
 }
 
 OptimizationRemarkMissed::OptimizationRemarkMissed(
@@ -303,9 +250,10 @@ OptimizationRemarkMissed::OptimizationRemarkMissed(const char *PassName,
                                    *Inst->getParent()->getParent(),
                                    Inst->getDebugLoc(), Inst->getParent()) {}
 
-bool OptimizationRemarkMissed::isEnabled(StringRef PassName) {
-  return PassRemarksMissedOptLoc.Pattern &&
-         PassRemarksMissedOptLoc.Pattern->match(PassName);
+bool OptimizationRemarkMissed::isEnabled() const {
+  const Function &Fn = getFunction();
+  LLVMContext &Ctx = Fn.getContext();
+  return Ctx.getDiagHandlerPtr()->isMissedOptRemarkEnabled(getPassName());
 }
 
 OptimizationRemarkAnalysis::OptimizationRemarkAnalysis(
@@ -330,9 +278,11 @@ OptimizationRemarkAnalysis::OptimizationRemarkAnalysis(
                                    *cast<BasicBlock>(CodeRegion)->getParent(),
                                    Loc, CodeRegion) {}
 
-bool OptimizationRemarkAnalysis::isEnabled(StringRef PassName) {
-  return PassRemarksAnalysisOptLoc.Pattern &&
-         PassRemarksAnalysisOptLoc.Pattern->match(PassName);
+bool OptimizationRemarkAnalysis::isEnabled() const {
+  const Function &Fn = getFunction();
+  LLVMContext &Ctx = Fn.getContext();
+  return Ctx.getDiagHandlerPtr()->isAnalysisRemarkEnabled(getPassName()) ||
+         shouldAlwaysPrint();
 }
 
 void DiagnosticInfoMIRParser::print(DiagnosticPrinter &DP) const {

lib/IR/LLVMContext.cpp

@@ -129,11 +129,17 @@ void *LLVMContext::getInlineAsmDiagnosticContext() const {
   return pImpl->InlineAsmDiagContext;
 }
 
-void LLVMContext::setDiagnosticHandler(DiagnosticHandlerTy DiagnosticHandler,
-                                       void *DiagnosticContext,
-                                       bool RespectFilters) {
-  pImpl->DiagnosticHandler = DiagnosticHandler;
-  pImpl->DiagnosticContext = DiagnosticContext;
+void LLVMContext::setDiagnosticHandlerCallBack(
+    DiagnosticHandler::DiagnosticHandlerTy DiagnosticHandler,
+    void *DiagnosticContext, bool RespectFilters) {
+  pImpl->DiagHandler->DiagHandlerCallback = DiagnosticHandler;
+  pImpl->DiagHandler->DiagnosticContext = DiagnosticContext;
+  pImpl->RespectDiagnosticFilters = RespectFilters;
+}
+
+void LLVMContext::setDiagnosticHandler(std::unique_ptr<DiagnosticHandler> &&DH,
+                                      bool RespectFilters) {
+  pImpl->DiagHandler = std::move(DH);
   pImpl->RespectDiagnosticFilters = RespectFilters;
 }
 
@@ -159,12 +165,13 @@ void LLVMContext::setDiagnosticsOutputFile(std::unique_ptr<yaml::Output> F) {
   pImpl->DiagnosticsOutputFile = std::move(F);
 }
 
-LLVMContext::DiagnosticHandlerTy LLVMContext::getDiagnosticHandler() const {
-  return pImpl->DiagnosticHandler;
+DiagnosticHandler::DiagnosticHandlerTy
+LLVMContext::getDiagnosticHandlerCallBack() const {
+  return pImpl->DiagHandler->DiagHandlerCallback;
 }
 
 void *LLVMContext::getDiagnosticContext() const {
-  return pImpl->DiagnosticContext;
+  return pImpl->DiagHandler->DiagnosticContext;
 }
 
 void LLVMContext::setYieldCallback(YieldCallbackTy Callback, void *OpaqueHandle)
@@ -215,11 +222,10 @@ LLVMContext::getDiagnosticMessagePrefix(DiagnosticSeverity Severity) {
 
 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);
+  if (pImpl->DiagHandler &&
+      (!pImpl->RespectDiagnosticFilters || isDiagnosticEnabled(DI)) &&
+      pImpl->DiagHandler->handleDiagnostics(DI))
     return;
-  }
 
   if (!isDiagnosticEnabled(DI))
     return;
@@ -315,3 +321,11 @@ void LLVMContext::setDiscardValueNames(bool Discard) {
 OptBisect &LLVMContext::getOptBisect() {
   return pImpl->getOptBisect();
 }
+
+const DiagnosticHandler *LLVMContext::getDiagHandlerPtr() const {
+  return pImpl->DiagHandler.get();
+}
+
+std::unique_ptr<DiagnosticHandler> LLVMContext::getDiagnosticHandler() {
+  return std::move(pImpl->DiagHandler);
+}

lib/IR/LLVMContextImpl.cpp

@@ -22,7 +22,8 @@
 using namespace llvm;
 
 LLVMContextImpl::LLVMContextImpl(LLVMContext &C)
-  : VoidTy(C, Type::VoidTyID),
+  : DiagHandler(llvm::make_unique<DiagnosticHandler>()),
+    VoidTy(C, Type::VoidTyID),
     LabelTy(C, Type::LabelTyID),
     HalfTy(C, Type::HalfTyID),
     FloatTy(C, Type::FloatTyID),

lib/IR/LLVMContextImpl.h

@@ -1168,8 +1168,7 @@ public:
   LLVMContext::InlineAsmDiagHandlerTy InlineAsmDiagHandler = nullptr;
   void *InlineAsmDiagContext = nullptr;
 
-  LLVMContext::DiagnosticHandlerTy DiagnosticHandler = nullptr;
-  void *DiagnosticContext = nullptr;
+  std::unique_ptr<DiagnosticHandler> DiagHandler;
   bool RespectDiagnosticFilters = false;
   bool DiagnosticsHotnessRequested = false;
   uint64_t DiagnosticsHotnessThreshold = 0;

lib/LTO/LTOCodeGenerator.cpp

@@ -622,12 +622,8 @@ void LTOCodeGenerator::parseCodeGenDebugOptions() {
   }
 }
 
-void LTOCodeGenerator::DiagnosticHandler(const DiagnosticInfo &DI,
-                                         void *Context) {
-  ((LTOCodeGenerator *)Context)->DiagnosticHandler2(DI);
-}
 
-void LTOCodeGenerator::DiagnosticHandler2(const DiagnosticInfo &DI) {
+void LTOCodeGenerator::DiagnosticHandler(const DiagnosticInfo &DI) {
   // Map the LLVM internal diagnostic severity to the LTO diagnostic severity.
   lto_codegen_diagnostic_severity_t Severity;
   switch (DI.getSeverity()) {
@@ -657,17 +653,29 @@ void LTOCodeGenerator::DiagnosticHandler2(const DiagnosticInfo &DI) {
   (*DiagHandler)(Severity, MsgStorage.c_str(), DiagContext);
 }
 
+namespace {
+struct LTODiagnosticHandler : public DiagnosticHandler {
+  LTOCodeGenerator *CodeGenerator;
+  LTODiagnosticHandler(LTOCodeGenerator *CodeGenPtr)
+      : CodeGenerator(CodeGenPtr) {}
+  bool handleDiagnostics(const DiagnosticInfo &DI) override {
+    CodeGenerator->DiagnosticHandler(DI);
+    return true;
+  }
+};
+}
+
 void
 LTOCodeGenerator::setDiagnosticHandler(lto_diagnostic_handler_t DiagHandler,
                                        void *Ctxt) {
   this->DiagHandler = DiagHandler;
   this->DiagContext = Ctxt;
   if (!DiagHandler)
-    return Context.setDiagnosticHandler(nullptr, nullptr);
+    return Context.setDiagnosticHandler(nullptr);
   // Register the LTOCodeGenerator stub in the LLVMContext to forward the
   // diagnostic to the external DiagHandler.
-  Context.setDiagnosticHandler(LTOCodeGenerator::DiagnosticHandler, this,
-                               /* RespectFilters */ true);
+  Context.setDiagnosticHandler(llvm::make_unique<LTODiagnosticHandler>(this),
+                               true);
 }
 
 namespace {

lib/Transforms/Scalar/GVN.cpp

@@ -920,8 +920,7 @@ bool GVN::AnalyzeLoadAvailability(LoadInst *LI, MemDepResult DepInfo,
       Instruction *I = DepInfo.getInst();
       dbgs() << " is clobbered by " << *I << '\n';
     );
-
-    if (ORE->allowExtraAnalysis())
+    if (ORE->allowExtraAnalysis(DEBUG_TYPE))
       reportMayClobberedLoad(LI, DepInfo, DT, ORE);
 
     return false;

lib/Transforms/Vectorize/LoopVectorize.cpp

@@ -4957,12 +4957,15 @@ bool LoopVectorizationLegality::canVectorize() {
   // Store the result and return it at the end instead of exiting early, in case
   // allowExtraAnalysis is used to report multiple reasons for not vectorizing.
   bool Result = true;
+  
+  bool DoExtraAnalysis = ORE->allowExtraAnalysis(DEBUG_TYPE);
+  if (DoExtraAnalysis)
   // We must have a loop in canonical form. Loops with indirectbr in them cannot
   // be canonicalized.
   if (!TheLoop->getLoopPreheader()) {
     ORE->emit(createMissedAnalysis("CFGNotUnderstood")
               << "loop control flow is not understood by vectorizer");
-    if (ORE->allowExtraAnalysis())
+  if (DoExtraAnalysis)
       Result = false;
     else
       return false;
@@ -4975,7 +4978,7 @@ bool LoopVectorizationLegality::canVectorize() {
   if (!TheLoop->empty()) {
     ORE->emit(createMissedAnalysis("NotInnermostLoop")
               << "loop is not the innermost loop");
-    if (ORE->allowExtraAnalysis())
+    if (DoExtraAnalysis)
       Result = false;
     else
       return false;
@@ -4985,7 +4988,7 @@ bool LoopVectorizationLegality::canVectorize() {
   if (TheLoop->getNumBackEdges() != 1) {
     ORE->emit(createMissedAnalysis("CFGNotUnderstood")
               << "loop control flow is not understood by vectorizer");
-    if (ORE->allowExtraAnalysis())
+    if (DoExtraAnalysis)
       Result = false;
     else
       return false;
@@ -4995,7 +4998,7 @@ bool LoopVectorizationLegality::canVectorize() {
   if (!TheLoop->getExitingBlock()) {
     ORE->emit(createMissedAnalysis("CFGNotUnderstood")
               << "loop control flow is not understood by vectorizer");
-    if (ORE->allowExtraAnalysis())
+    if (DoExtraAnalysis)
       Result = false;
     else
       return false;
@@ -5007,7 +5010,7 @@ bool LoopVectorizationLegality::canVectorize() {
   if (TheLoop->getExitingBlock() != TheLoop->getLoopLatch()) {
     ORE->emit(createMissedAnalysis("CFGNotUnderstood")
               << "loop control flow is not understood by vectorizer");
-    if (ORE->allowExtraAnalysis())
+    if (DoExtraAnalysis)
       Result = false;
     else
       return false;
@@ -5021,7 +5024,7 @@ bool LoopVectorizationLegality::canVectorize() {
   unsigned NumBlocks = TheLoop->getNumBlocks();
   if (NumBlocks != 1 && !canVectorizeWithIfConvert()) {
     DEBUG(dbgs() << "LV: Can't if-convert the loop.\n");
-    if (ORE->allowExtraAnalysis())
+    if (DoExtraAnalysis)
       Result = false;
     else
       return false;
@@ -5030,7 +5033,7 @@ bool LoopVectorizationLegality::canVectorize() {
   // Check if we can vectorize the instructions and CFG in this loop.
   if (!canVectorizeInstrs()) {
     DEBUG(dbgs() << "LV: Can't vectorize the instructions or CFG\n");
-    if (ORE->allowExtraAnalysis())
+    if (DoExtraAnalysis)
       Result = false;
     else
       return false;
@@ -5039,7 +5042,7 @@ bool LoopVectorizationLegality::canVectorize() {
   // Go over each instruction and look at memory deps.
   if (!canVectorizeMemory()) {
     DEBUG(dbgs() << "LV: Can't vectorize due to memory conflicts\n");
-    if (ORE->allowExtraAnalysis())
+    if (DoExtraAnalysis)
       Result = false;
     else
       return false;
@@ -5070,7 +5073,7 @@ bool LoopVectorizationLegality::canVectorize() {
               << "Too many SCEV assumptions need to be made and checked "
               << "at runtime");
     DEBUG(dbgs() << "LV: Too many SCEV checks needed.\n");
-    if (ORE->allowExtraAnalysis())
+    if (DoExtraAnalysis)
       Result = false;
     else
       return false;

test/Transforms/GVN/opt-remarks.ll

@@ -11,7 +11,7 @@
 ; CHECK:      remark: <unknown>:0:0: load of type i32 eliminated{{$}}
 ; CHECK-NEXT: remark: <unknown>:0:0: load of type i32 eliminated{{$}}
 ; CHECK-NEXT: remark: <unknown>:0:0: load of type i32 eliminated{{$}}
-; CHECK-NOT:  remark:
+; CHECK-NEXT: remark: /tmp/s.c:3:3: load of type i32 not eliminated
 
 ; YAML:      --- !Passed
 ; YAML-NEXT: Pass:            gvn

tools/llc/llc.cpp

@@ -235,20 +235,24 @@ GetOutputStream(const char *TargetName, Triple::OSType OS,
   return FDOut;
 }
 
-static void DiagnosticHandler(const DiagnosticInfo &DI, void *Context) {
-  bool *HasError = static_cast<bool *>(Context);
-  if (DI.getSeverity() == DS_Error)
-    *HasError = true;
+struct LLCDiagnosticHandler : public DiagnosticHandler {
+  bool *HasError;
+  LLCDiagnosticHandler(bool *HasErrorPtr) : HasError(HasErrorPtr) {}
+  bool handleDiagnostics(const DiagnosticInfo &DI) override {
+    if (DI.getSeverity() == DS_Error)
+      *HasError = true;
 
-  if (auto *Remark = dyn_cast<DiagnosticInfoOptimizationBase>(&DI))
-    if (!Remark->isEnabled())
-      return;
+    if (auto *Remark = dyn_cast<DiagnosticInfoOptimizationBase>(&DI))
+      if (!Remark->isEnabled())
+        return true;
 
-  DiagnosticPrinterRawOStream DP(errs());
-  errs() << LLVMContext::getDiagnosticMessagePrefix(DI.getSeverity()) << ": ";
-  DI.print(DP);
-  errs() << "\n";
-}
+    DiagnosticPrinterRawOStream DP(errs());
+    errs() << LLVMContext::getDiagnosticMessagePrefix(DI.getSeverity()) << ": ";
+    DI.print(DP);
+    errs() << "\n";
+    return true;
+  }
+};
 
 static void InlineAsmDiagHandler(const SMDiagnostic &SMD, void *Context,
                                  unsigned LocCookie) {
@@ -308,7 +312,8 @@ int main(int argc, char **argv) {
 
   // Set a diagnostic handler that doesn't exit on the first error
   bool HasError = false;
-  Context.setDiagnosticHandler(DiagnosticHandler, &HasError);
+  Context.setDiagnosticHandler(
+      llvm::make_unique<LLCDiagnosticHandler>(&HasError));
   Context.setInlineAsmDiagnosticHandler(InlineAsmDiagHandler, &HasError);
 
   if (PassRemarksWithHotness)
@@ -564,8 +569,9 @@ static int compileModule(char **argv, LLVMContext &Context) {
 
     PM.run(*M);
 
-    auto HasError = *static_cast<bool *>(Context.getDiagnosticContext());
-    if (HasError)
+    auto HasError =
+        ((const LLCDiagnosticHandler *)(Context.getDiagHandlerPtr()))->HasError;
+    if (*HasError)
       return 1;
 
     // Compare the two outputs and make sure they're the same

tools/llvm-dis/llvm-dis.cpp

@@ -122,25 +122,29 @@ public:
   }
 };
 
-} // end anon namespace
+struct LLVMDisDiagnosticHandler : public DiagnosticHandler {
+  char *Prefix;
+  LLVMDisDiagnosticHandler(char *PrefixPtr) : Prefix(PrefixPtr) {}
+  bool handleDiagnostics(const DiagnosticInfo &DI) override {
+    raw_ostream &OS = errs();
+    OS << Prefix << ": ";
+    switch (DI.getSeverity()) {
+      case DS_Error: OS << "error: "; break;
+      case DS_Warning: OS << "warning: "; break;
+      case DS_Remark: OS << "remark: "; break;
+      case DS_Note: OS << "note: "; break;
+    }
 
-static void diagnosticHandler(const DiagnosticInfo &DI, void *Context) {
-  raw_ostream &OS = errs();
-  OS << (char *)Context << ": ";
-  switch (DI.getSeverity()) {
-  case DS_Error: OS << "error: "; break;
-  case DS_Warning: OS << "warning: "; break;
-  case DS_Remark: OS << "remark: "; break;
-  case DS_Note: OS << "note: "; break;
+    DiagnosticPrinterRawOStream DP(OS);
+    DI.print(DP);
+    OS << '\n';
+
+    if (DI.getSeverity() == DS_Error)
+      exit(1);
+    return true;
   }
-
-  DiagnosticPrinterRawOStream DP(OS);
-  DI.print(DP);
-  OS << '\n';
-
-  if (DI.getSeverity() == DS_Error)
-    exit(1);
-}
+};
+} // end anon namespace
 
 static ExitOnError ExitOnErr;
 
@@ -166,9 +170,8 @@ int main(int argc, char **argv) {
 
   LLVMContext Context;
   llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
-
-  Context.setDiagnosticHandler(diagnosticHandler, argv[0]);
-
+  Context.setDiagnosticHandler(
+      llvm::make_unique<LLVMDisDiagnosticHandler>(argv[0]));
   cl::ParseCommandLineOptions(argc, argv, "llvm .bc -> .ll disassembler\n");
 
   std::unique_ptr<Module> M = openInputFile(Context);

tools/llvm-link/llvm-link.cpp

@@ -182,25 +182,30 @@ Module &ModuleLazyLoaderCache::operator()(const char *argv0,
 }
 } // anonymous namespace
 
-static void diagnosticHandler(const DiagnosticInfo &DI, void *C) {
-  unsigned Severity = DI.getSeverity();
-  switch (Severity) {
-  case DS_Error:
-    errs() << "ERROR: ";
-    break;
-  case DS_Warning:
-    if (SuppressWarnings)
-      return;
-    errs() << "WARNING: ";
-    break;
-  case DS_Remark:
-  case DS_Note:
-    llvm_unreachable("Only expecting warnings and errors");
-  }
+namespace {
+struct LLVMLinkDiagnosticHandler : public DiagnosticHandler {
+  bool handleDiagnostics(const DiagnosticInfo &DI) override {
+    unsigned Severity = DI.getSeverity();
+    switch (Severity) {
+    case DS_Error:
+      errs() << "ERROR: ";
+      break;
+    case DS_Warning:
+      if (SuppressWarnings)
+        return true;
+      errs() << "WARNING: ";
+      break;
+    case DS_Remark:
+    case DS_Note:
+      llvm_unreachable("Only expecting warnings and errors");
+    }
 
-  DiagnosticPrinterRawOStream DP(errs());
-  DI.print(DP);
-  errs() << '\n';
+    DiagnosticPrinterRawOStream DP(errs());
+    DI.print(DP);
+    errs() << '\n';
+    return true;
+  }
+};
 }
 
 /// Import any functions requested via the -import option.
@@ -347,8 +352,8 @@ int main(int argc, char **argv) {
   ExitOnErr.setBanner(std::string(argv[0]) + ": ");
 
   LLVMContext Context;
-  Context.setDiagnosticHandler(diagnosticHandler, nullptr, true);
-
+  Context.setDiagnosticHandler(
+    llvm::make_unique<LLVMLinkDiagnosticHandler>(), true);
   llvm_shutdown_obj Y;  // Call llvm_shutdown() on exit.
   cl::ParseCommandLineOptions(argc, argv, "llvm linker\n");
 

tools/llvm-lto/llvm-lto.cpp

@@ -235,34 +235,40 @@ static void handleDiagnostics(lto_codegen_diagnostic_severity_t Severity,
 }
 
 static std::string CurrentActivity;
-static void diagnosticHandler(const DiagnosticInfo &DI, void *Context) {
-  raw_ostream &OS = errs();
-  OS << "llvm-lto: ";
-  switch (DI.getSeverity()) {
-  case DS_Error:
-    OS << "error";
-    break;
-  case DS_Warning:
-    OS << "warning";
-    break;
-  case DS_Remark:
-    OS << "remark";
-    break;
-  case DS_Note:
-    OS << "note";
-    break;
+
+namespace {
+  struct LLVMLTODiagnosticHandler : public DiagnosticHandler {
+    bool handleDiagnostics(const DiagnosticInfo &DI) override {
+      raw_ostream &OS = errs();
+      OS << "llvm-lto: ";
+      switch (DI.getSeverity()) {
+      case DS_Error:
+        OS << "error";
+        break;
+      case DS_Warning:
+        OS << "warning";
+        break;
+      case DS_Remark:
+        OS << "remark";
+        break;
+      case DS_Note:
+        OS << "note";
+        break;
+      }
+      if (!CurrentActivity.empty())
+        OS << ' ' << CurrentActivity;
+      OS << ": ";
+  
+      DiagnosticPrinterRawOStream DP(OS);
+      DI.print(DP);
+      OS << '\n';
+  
+      if (DI.getSeverity() == DS_Error)
+        exit(1);
+      return true;
+    }
+  };
   }
-  if (!CurrentActivity.empty())
-    OS << ' ' << CurrentActivity;
-  OS << ": ";
-
-  DiagnosticPrinterRawOStream DP(OS);
-  DI.print(DP);
-  OS << '\n';
-
-  if (DI.getSeverity() == DS_Error)
-    exit(1);
-}
 
 static void error(const Twine &Msg) {
   errs() << "llvm-lto: " << Msg << '\n';
@@ -293,7 +299,8 @@ getLocalLTOModule(StringRef Path, std::unique_ptr<MemoryBuffer> &Buffer,
   Buffer = std::move(BufferOrErr.get());
   CurrentActivity = ("loading file '" + Path + "'").str();
   std::unique_ptr<LLVMContext> Context = llvm::make_unique<LLVMContext>();
-  Context->setDiagnosticHandler(diagnosticHandler, nullptr, true);
+  Context->setDiagnosticHandler(llvm::make_unique<LLVMLTODiagnosticHandler>(),
+                                true);
   ErrorOr<std::unique_ptr<LTOModule>> Ret = LTOModule::createInLocalContext(
       std::move(Context), Buffer->getBufferStart(), Buffer->getBufferSize(),
       Options, Path);
@@ -837,7 +844,8 @@ int main(int argc, char **argv) {
   unsigned BaseArg = 0;
 
   LLVMContext Context;
-  Context.setDiagnosticHandler(diagnosticHandler, nullptr, true);
+  Context.setDiagnosticHandler(llvm::make_unique<LLVMLTODiagnosticHandler>(),
+                               true);
 
   LTOCodeGenerator CodeGen(Context);
 

tools/lto/lto.cpp

@@ -75,20 +75,23 @@ static bool parsedOptions = false;
 
 static LLVMContext *LTOContext = nullptr;
 
-static void diagnosticHandler(const DiagnosticInfo &DI, void *Context) {
-  if (DI.getSeverity() != DS_Error) {
-    DiagnosticPrinterRawOStream DP(errs());
-    DI.print(DP);
-    errs() << '\n';
-    return;
+struct LTOToolDiagnosticHandler : public DiagnosticHandler {
+  bool handleDiagnostics(const DiagnosticInfo &DI) override {
+    if (DI.getSeverity() != DS_Error) {
+      DiagnosticPrinterRawOStream DP(errs());
+      DI.print(DP);
+      errs() << '\n';
+      return true;
+    }
+    sLastErrorString = "";
+    {
+      raw_string_ostream Stream(sLastErrorString);
+      DiagnosticPrinterRawOStream DP(Stream);
+      DI.print(DP);
+    }
+    return true;
   }
-  sLastErrorString = "";
-  {
-    raw_string_ostream Stream(sLastErrorString);
-    DiagnosticPrinterRawOStream DP(Stream);
-    DI.print(DP);
-  }
-}
+};
 
 // Initialize the configured targets if they have not been initialized.
 static void lto_initialize() {
@@ -108,7 +111,8 @@ static void lto_initialize() {
 
     static LLVMContext Context;
     LTOContext = &Context;
-    LTOContext->setDiagnosticHandler(diagnosticHandler, nullptr, true);
+    LTOContext->setDiagnosticHandler(
+        llvm::make_unique<LTOToolDiagnosticHandler>(), true);
     initialized = true;
   }
 }
@@ -274,7 +278,8 @@ lto_module_t lto_module_create_in_local_context(const void *mem, size_t length,
 
   // Create a local context. Ownership will be transferred to LTOModule.
   std::unique_ptr<LLVMContext> Context = llvm::make_unique<LLVMContext>();
-  Context->setDiagnosticHandler(diagnosticHandler, nullptr, true);
+  Context->setDiagnosticHandler(llvm::make_unique<LTOToolDiagnosticHandler>(),
+                                true);
 
   ErrorOr<std::unique_ptr<LTOModule>> M = LTOModule::createInLocalContext(
       std::move(Context), mem, length, Options, StringRef(path));