[Flang][Driver] Add location and remark option printing to R_Group Diagnostics

For each R_Group diagnostic produced, this patch gives more
information about it by printing the absolute file path,
the line and column number the pass was applied to and finally
the remark option that was used.

Clang does the same with the exception of printing the relative
path rather than absolute path.

Depends on D159260. That patch adds support for backend passes
while this patch adds remark options to the backend test cases.

Reviewed By: awarzynski

Differential Revision: https://reviews.llvm.org/D159258

Author: victorkingi

flang/include/flang/Frontend/TextDiagnosticPrinter.h

@@ -52,6 +52,9 @@ class TextDiagnosticPrinter : public clang::DiagnosticConsumer {
 
   void HandleDiagnostic(clang::DiagnosticsEngine::Level level,
                         const clang::Diagnostic &info) override;
+
+  void printLocForRemarks(llvm::raw_svector_ostream &diagMessageStream,
+                          llvm::StringRef &diagMsg);
 };
 
 } // namespace Fortran::frontend

flang/lib/Frontend/CompilerInvocation.cpp

@@ -267,6 +267,21 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
       diags, args, clang::driver::options::OPT_Rpass_analysis_EQ,
       /*remarkOptName=*/"pass-analysis");
 
+  if (opts.getDebugInfo() == llvm::codegenoptions::NoDebugInfo) {
+    // If the user requested a flag that requires source locations available in
+    // the backend, make sure that the backend tracks source location
+    // information.
+    bool needLocTracking = !opts.OptRecordFile.empty() ||
+                           !opts.OptRecordPasses.empty() ||
+                           !opts.OptRecordFormat.empty() ||
+                           opts.OptimizationRemark.hasValidPattern() ||
+                           opts.OptimizationRemarkMissed.hasValidPattern() ||
+                           opts.OptimizationRemarkAnalysis.hasValidPattern();
+
+    if (needLocTracking)
+      opts.setDebugInfo(llvm::codegenoptions::LocTrackingOnly);
+  }
+
   if (auto *a = args.getLastArg(clang::driver::options::OPT_save_temps_EQ))
     opts.SaveTempsDir = a->getValue();
 

flang/lib/Frontend/FrontendActions.cpp

@@ -957,6 +957,19 @@ class BackendRemarkConsumer : public llvm::DiagnosticHandler {
 
     std::string msg;
     llvm::raw_string_ostream msgStream(msg);
+
+    if (diagInfo.isLocationAvailable()) {
+      // Clang contains a SourceManager class which handles loading
+      // and caching of source files into memory and it can be used to
+      // query SourceLocation data. The SourceLocation data is what is
+      // needed here as it contains the full include stack which gives
+      // line and column number as well as file name and location.
+      // Since Flang doesn't have SourceManager, send file name and absolute
+      // path through msgStream, to use for printing.
+      msgStream << diagInfo.getLocationStr() << ";;"
+                << diagInfo.getAbsolutePath() << ";;";
+    }
+
     msgStream << diagInfo.getMsg();
 
     // Emit message.

flang/lib/Frontend/TextDiagnosticPrinter.cpp

@@ -18,7 +18,10 @@
 #include "flang/Frontend/TextDiagnostic.h"
 #include "clang/Basic/DiagnosticOptions.h"
 #include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
 using namespace Fortran::frontend;
@@ -29,6 +32,62 @@ TextDiagnosticPrinter::TextDiagnosticPrinter(raw_ostream &diagOs,
 
 TextDiagnosticPrinter::~TextDiagnosticPrinter() {}
 
+// For remarks only, print the remark option and pass name that was used to a
+// raw_ostream. This also supports warnings from invalid remark arguments
+// provided.
+static void printRemarkOption(llvm::raw_ostream &os,
+                              clang::DiagnosticsEngine::Level level,
+                              const clang::Diagnostic &info) {
+  llvm::StringRef opt =
+      clang::DiagnosticIDs::getWarningOptionForDiag(info.getID());
+  if (!opt.empty()) {
+    // We still need to check if the level is a Remark since, an unknown option
+    // warning could be printed i.e. [-Wunknown-warning-option]
+    os << " [" << (level == clang::DiagnosticsEngine::Remark ? "-R" : "-W")
+       << opt;
+    llvm::StringRef optValue = info.getDiags()->getFlagValue();
+    if (!optValue.empty())
+      os << "=" << optValue;
+    os << ']';
+  }
+}
+
+// For remarks only, if we are receiving a message of this format
+// [file location with line and column];;[path to file];;[the remark message]
+// then print the absolute file path, line and column number.
+void TextDiagnosticPrinter::printLocForRemarks(
+    llvm::raw_svector_ostream &diagMessageStream, llvm::StringRef &diagMsg) {
+  // split incoming string to get the absolute path and filename in the
+  // case we are receiving optimization remarks from BackendRemarkConsumer
+  diagMsg = diagMessageStream.str();
+  llvm::StringRef delimiter = ";;";
+
+  size_t pos = 0;
+  llvm::SmallVector<llvm::StringRef> tokens;
+  while ((pos = diagMsg.find(delimiter)) != std::string::npos) {
+    tokens.push_back(diagMsg.substr(0, pos));
+    diagMsg = diagMsg.drop_front(pos + delimiter.size());
+  }
+
+  // tokens will always be of size 2 in the case of optimization
+  // remark message received
+  if (tokens.size() == 2) {
+    // Extract absolute path
+    llvm::SmallString<128> absPath = llvm::sys::path::relative_path(tokens[1]);
+    llvm::sys::path::remove_filename(absPath);
+    // Add the last separator before the file name
+    llvm::sys::path::append(absPath, llvm::sys::path::get_separator());
+    llvm::sys::path::make_preferred(absPath);
+
+    // Used for changing only the bold attribute
+    if (diagOpts->ShowColors)
+      os.changeColor(llvm::raw_ostream::SAVEDCOLOR, true);
+
+    // Print path, file name, line and column
+    os << absPath << tokens[0] << ": ";
+  }
+}
+
 void TextDiagnosticPrinter::HandleDiagnostic(
     clang::DiagnosticsEngine::Level level, const clang::Diagnostic &info) {
   // Default implementation (Warnings/errors count).
@@ -40,6 +99,7 @@ void TextDiagnosticPrinter::HandleDiagnostic(
   info.FormatDiagnostic(outStr);
 
   llvm::raw_svector_ostream diagMessageStream(outStr);
+  printRemarkOption(diagMessageStream, level, info);
 
   if (!prefix.empty())
     os << prefix << ": ";
@@ -48,12 +108,15 @@ void TextDiagnosticPrinter::HandleDiagnostic(
   assert(!info.getLocation().isValid() &&
          "Diagnostics with valid source location are not supported");
 
+  llvm::StringRef diagMsg;
+  printLocForRemarks(diagMessageStream, diagMsg);
+
   Fortran::frontend::TextDiagnostic::printDiagnosticLevel(os, level,
                                                           diagOpts->ShowColors);
   Fortran::frontend::TextDiagnostic::printDiagnosticMessage(
       os,
-      /*IsSupplemental=*/level == clang::DiagnosticsEngine::Note,
-      diagMessageStream.str(), diagOpts->ShowColors);
+      /*IsSupplemental=*/level == clang::DiagnosticsEngine::Note, diagMsg,
+      diagOpts->ShowColors);
 
   os.flush();
 }

flang/test/Driver/optimization-remark-backend.f90

@@ -9,8 +9,9 @@
 ! Check full -Rpass-analysis message is emitted
 ! RUN: %flang %s -O1 -Rpass-analysis %{output} 2>&1 | FileCheck %s --check-prefix=ANALYSIS
 
-! MISSED:   remark: {{[0-9]+}} virtual registers copies {{.*}} total copies cost generated in function
+! MISSED:   remark: {{[0-9]+}} virtual registers copies {{.*}} total copies cost generated in function [-Rpass-missed=regalloc]
 ! ANALYSIS: remark: BasicBlock:
+! ANALYSIS: [-Rpass-analysis=asm-printer]
 
 program forttest
     implicit none

flang/test/Driver/optimization-remark-invalid.f90

@@ -12,8 +12,8 @@
 ! RUN: %flang %s -O1 -Rpas -c 2>&1 | FileCheck %s --check-prefix=WARN-SUGGEST
 
 ! REGEX-INVALID: error: in pattern '-Rpass=[': brackets ([ ]) not balanced
-! WARN: warning: unknown remark option '-R'
-! WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'?
+! WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
 
 program forttest
 end program forttest

flang/test/Driver/optimization-remark.f90

@@ -40,29 +40,29 @@
 ! With plain -Rpass, -Rpass-missed or -Rpass-analysis, we expect remarks related to 2 opportunities (loop vectorisation / loop delete and load hoisting).
 ! Once we start filtering, this is reduced to 1 one of the loop passes.
 
-! PASS-REGEX-LOOP-ONLY-NOT:     remark: hoisting load
-! PASS-REGEX-LOOP-ONLY:         remark: Loop deleted because it is invariant
+! PASS-REGEX-LOOP-ONLY-NOT:     optimization-remark.f90:77:7: remark: hoisting load [-Rpass=licm]
+! PASS-REGEX-LOOP-ONLY:         optimization-remark.f90:83:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
 
-! MISSED-REGEX-LOOP-ONLY-NOT:   remark: failed to hoist load with loop-invariant address because load is conditionally executed
-! MISSED-REGEX-LOOP-ONLY:       remark: loop not vectorized
+! MISSED-REGEX-LOOP-ONLY-NOT:   optimization-remark.f90:77:7: remark: failed to hoist load with loop-invariant address because load is conditionally executed [-Rpass-missed=licm]
+! MISSED-REGEX-LOOP-ONLY:       optimization-remark.f90:76:4: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
 
 
-! ANALYSIS-REGEX-LOOP-ONLY:     remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
-! ANALYSIS-REGEX-LOOP-ONLY:     Unknown data dependence.
-! ANALYSIS-REGEX-LOOP-ONLY-NOT: remark:{{.*}}: IR instruction count changed from {{[0-9]+}} to {{[0-9]+}}; Delta: {{-?[0-9]+}}
+! ANALYSIS-REGEX-LOOP-ONLY:     optimization-remark.f90:79:7: remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
+! ANALYSIS-REGEX-LOOP-ONLY:     Unknown data dependence. Memory location is the same as accessed at optimization-remark.f90:78:7 [-Rpass-analysis=loop-vectorize]
+! ANALYSIS-REGEX-LOOP-ONLY-NOT: remark: {{.*}}: IR instruction count changed from {{[0-9]+}} to {{[0-9]+}}; Delta: {{-?[0-9]+}} [-Rpass-analysis=size-info]
 
-! PASS:                         remark: hoisting load
-! PASS:                         remark: Loop deleted because it is invariant
+! PASS:                         optimization-remark.f90:77:7: remark: hoisting load [-Rpass=licm]
+! PASS:                         optimization-remark.f90:83:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
 
-! MISSED:                       remark: failed to hoist load with loop-invariant address because load is conditionally executed
-! MISSED:                       remark: loop not vectorized
-! MISSED-NOT:                   remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
-! MISSED-NOT:                   Unknown data dependence.
+! MISSED:                       optimization-remark.f90:77:7: remark: failed to hoist load with loop-invariant address because load is conditionally executed [-Rpass-missed=licm]
+! MISSED:                       optimization-remark.f90:76:4: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! MISSED-NOT:                   optimization-remark.f90:79:7: remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
+! MISSED-NOT:                   Unknown data dependence. Memory location is the same as accessed at optimization-remark.f90:78:7 [-Rpass-analysis=loop-vectorize]
 
-! ANALYSIS:                     remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
-! ANALYSIS:                     Unknown data dependence.
-! ANALYSIS:                     remark: {{.*}}: IR instruction count changed from {{[0-9]+}} to {{[0-9]+}}; Delta: {{-?[0-9]+}}
-! ANALYSIS-NOT:                 remark: failed to hoist load with loop-invariant address because load is conditionally executed
+! ANALYSIS:                     optimization-remark.f90:79:7: remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
+! ANALYSIS:                     Unknown data dependence. Memory location is the same as accessed at optimization-remark.f90:78:7 [-Rpass-analysis=loop-vectorize]
+! ANALYSIS:                     remark: {{.*}}: IR instruction count changed from {{[0-9]+}} to {{[0-9]+}}; Delta: {{-?[0-9]+}} [-Rpass-analysis=size-info]
+! ANALYSIS-NOT:                 optimization-remark.f90:77:7: remark: failed to hoist load with loop-invariant address because load is conditionally executed [-Rpass-missed=licm]
 
 subroutine swap_real(a1, a2)
    implicit none