Skip to content

Commit

Permalink
[Flang][Driver] Add location and remark option printing to R_Group Di…
Browse files Browse the repository at this point in the history
…agnostics

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
  • Loading branch information
victorkingi committed Aug 31, 2023
1 parent 380c5da commit 12da8ef
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 22 deletions.
3 changes: 3 additions & 0 deletions flang/include/flang/Frontend/TextDiagnosticPrinter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions flang/lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
13 changes: 13 additions & 0 deletions flang/lib/Frontend/FrontendActions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
67 changes: 65 additions & 2 deletions flang/lib/Frontend/TextDiagnosticPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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).
Expand All @@ -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 << ": ";
Expand All @@ -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();
}
3 changes: 2 additions & 1 deletion flang/test/Driver/optimization-remark-backend.f90
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions flang/test/Driver/optimization-remark-invalid.f90
Original file line number Diff line number Diff line change
Expand Up @@ -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
34 changes: 17 additions & 17 deletions flang/test/Driver/optimization-remark.f90
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 12da8ef

Please sign in to comment.