Allow diagnostic handlers to check for optimization remarks.

[dnovillo]

Summary

When optimization remarks are enabled via the driver flag -Rpass, we
should allow the FE diagnostic handler to check if the given pass name
needs a diagnostic.

We were unconditionally checking the pattern defined in opt's
-pass-remarks flag. This was causing the FE to not emit any diagnostics.

[qcolombet]

Hi Diego,

I'd like we take another direction for this patch. See my inline comment.

Thanks,
-Quentin

lib/IR/LLVMContext.cpp

178

I am not a huge fan of duplicating this test (i.e., this is the test used in LLVMContext::diagnose).

Could we instead teach optimizationRemarksEnabledFor to always returns true when we set a diagnostic handler?

For instance, we could change the method used to check the optimization remarks within LLVMContext::setDiagnosticHandler.

[dnovillo]

lib/IR/LLVMContext.cpp

178

Oh, sure. I had initially done that, but it didn't seem like the predicate optimizationRemarksEnabledFor() was semantically equivalent if it also checks for a handler.

I'll sink the decision in LLVMContext::setDiagnosticHandler.

[dnovillo]

I did something slightly different. Setting a new function pointer
just for the optimization remarks seemed like overkill. I am doing
something similar to what is done in the front end.

The backend handler checks whether it's dealing with an optimization
remark. If so, it further checks the regexp passed in -pass-remarks.
This seemed less invasive than the additional function pointer.

[qcolombet]

Hi Diego,

That’s a good idea.

LGTM.

Thanks,
-Quentin

lib/IR/LLVMContext.cpp

 }
 
 void LLVMContext::diagnose(const DiagnosticInfo &DI) {
   // If there is a report handler, use it.
   if (pImpl->DiagnosticHandler) {
     pImpl->DiagnosticHandler(DI, pImpl->DiagnosticContext);
     return;
   }
+
+  // Optimization remarks are selective. They need to check whether
+  // the regexp pattern, passed via -pass-remarks, matches the name
+  // of the pass that is emitting the diagnostic. If there is no match,
+  // ignore the diagnostic and return.
+  if (DI.getKind() == llvm::DK_OptimizationRemark &&
+      !pImpl->optimizationRemarksEnabledFor(
+          cast<DiagnosticInfoOptimizationRemark>(DI).getPassName()))
+    return;
+
   // Otherwise, print the message with a prefix based on the severity.
   std::string MsgStorage;
   raw_string_ostream Stream(MsgStorage);
   DiagnosticPrinterRawOStream DP(Stream);
   DI.print(DP);
   Stream.flush();
   switch (DI.getSeverity()) {
   case DS_Error:

 void LLVMContext::emitError(unsigned LocCookie, const Twine &ErrorStr) {
   diagnose(DiagnosticInfoInlineAsm(LocCookie, ErrorStr));
 }
 
 void LLVMContext::emitOptimizationRemark(const char *PassName,
                                          const Function &Fn,
                                          const DebugLoc &DLoc,
                                          const Twine &Msg) {
-  if (pImpl->optimizationRemarksEnabledFor(PassName))
-    diagnose(DiagnosticInfoOptimizationRemark(PassName, Fn, DLoc, Msg));
 }
 
 //===----------------------------------------------------------------------===//
 // Metadata Kind Uniquing
 //===----------------------------------------------------------------------===//
 
 #ifndef NDEBUG
 /// isValidName - Return true if Name is a valid custom metadata handler name.
 static bool isValidName(StringRef MDName) {
   if (MDName.empty())
     return false;
 
   if (!std::isalpha(static_cast<unsigned char>(MDName[0])))