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

[victorkingi]

Summary

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.

[awarzynski]

Makes, made a few suggestions, thanks!

flang/lib/Frontend/FrontendActions.cpp

962

[nit] sourceMgr is a concept from Clang. Could you please clarify a bit for folks who only work with Flang. Something as simple as "Clang has X, Flang does not. Use this instead" would be OK.

flang/lib/Frontend/TextDiagnosticPrinter.cpp

111–140

Any chance for this to be a seperate function?

[victorkingi]

Moved absolute path printing to a function in TextDiagnosticPrinter.cpp
Replaced std::string with llvm::StringRef
Clarified comment in emitOptimizationMessage concerning SourceManager

[victorkingi]

Added warning option to invalid remark tests

[victorkingi]

added remark option to optimization-remark-backend.f90 test cases

[victorkingi]

no changes, rebasing

[awarzynski]

LGTM, thanks!

flang/lib/Frontend/FrontendActions.cpp

962–967

[victorkingi]

minor comment edit

flang/include/flang/Frontend/TextDiagnosticPrinter.h

public:
   ~TextDiagnosticPrinter() override;
 
   /// Set the diagnostic printer prefix string, which will be printed at the
   /// start of any diagnostics. If empty, no prefix string is used.
   void setPrefix(std::string value) { prefix = std::move(value); }
 
   void HandleDiagnostic(clang::DiagnosticsEngine::Level level,
                         const clang::Diagnostic &info) override;
+
+  void printLocForRemarks(llvm::raw_svector_ostream &diagMessageStream,
+                          llvm::StringRef &diagMsg);
 };
 
 } // namespace Fortran::frontend
 
 #endif

flang/lib/Frontend/CompilerInvocation.cpp

opts.OptimizationRemarkMissed = parseOptimizationRemark(
       /*remarkOptName=*/"pass-missed");
 
   // Create OptRemark that allows all optimization decisions made by LLVM
   // to be printed.
   opts.OptimizationRemarkAnalysis = parseOptimizationRemark(
       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();
 
   // -mrelocation-model option.
   if (const llvm::opt::Arg *a =
           args.getLastArg(clang::driver::options::OPT_mrelocation_model)) {
     llvm::StringRef modelName = a->getValue();
     auto relocModel =

flang/lib/Frontend/FrontendActions.cpp

public:
   emitOptimizationMessage(const llvm::DiagnosticInfoOptimizationBase &diagInfo,
                           unsigned diagID) {
     // We only support warnings and remarks.
     assert(diagInfo.getSeverity() == llvm::DS_Remark ||
            diagInfo.getSeverity() == llvm::DS_Warning);
 
     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.
     diags.Report(diagID) << clang::AddFlagValue(diagInfo.getPassName())
                          << msgStream.str();
   }
 
   void optimizationRemarkHandler(

flang/lib/Frontend/TextDiagnosticPrinter.cpp

 // Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/
 //
 //===----------------------------------------------------------------------===//
 
 #include "flang/Frontend/TextDiagnosticPrinter.h"
 #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;
 
 TextDiagnosticPrinter::TextDiagnosticPrinter(raw_ostream &diagOs,
                                              clang::DiagnosticOptions *diags)
     : os(diagOs), diagOpts(diags) {}
 
 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).
   DiagnosticConsumer::HandleDiagnostic(level, info);
 
   // Render the diagnostic message into a temporary buffer eagerly. We'll use
   // this later as we print out the diagnostic to the terminal.
   llvm::SmallString<100> outStr;
   info.FormatDiagnostic(outStr);
 
   llvm::raw_svector_ostream diagMessageStream(outStr);
+  printRemarkOption(diagMessageStream, level, info);
 
   if (!prefix.empty())
     os << prefix << ": ";
 
   // We only emit diagnostics in contexts that lack valid source locations.
   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

 ! This file tests backend passes emitted by the -Rpass family of flags
 ! loop-delete isn't enabled at O0 so we use at least O1
 
 ! DEFINE: %{output} = -S -o /dev/null 2>&1
 
 ! Check full -Rpass-missed message is emitted
 ! RUN: %flang %s -O1 -Rpass-missed %{output} 2>&1 | FileCheck %s --check-prefix=MISSED
 
 ! 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
     integer :: n
 
     do n = 1,2
         print *, ""
     end do
 
 end program forttest

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

 ! This file tests invalid usage of the -Rpass family of flags (-Rpass, -Rpass-missed
 ! and -Rpass-analysis)
 ! loop-delete isn't enabled at O0 so we use at least O1
 
 ! Check error on invalid regex -Rpass message is emitted
 ! RUN: not %flang %s -O1 -Rpass=[ -c 2>&1 | FileCheck %s --check-prefix=REGEX-INVALID
 
 ! Check "unknown remark option" warning
 ! RUN: %flang %s -O1 -R -c 2>&1 | FileCheck %s --check-prefix=WARN
 
 ! Check "unknown remark option" warning with suggestion
 ! 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

 
 ! REMARKS: remark:
 ! NO-REMARKS-NOT: remark:
 
 
 ! 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
 
    real, dimension(1:2) :: aR1
    integer :: i, n
    real, intent(inout) :: a1(:), a2(:)
    real :: a