[flang-commits] [flang] 12da8ef - [Flang][Driver] Add location and remark option printing to R_Group Diagnostics
Victor Kingi via flang-commits
flang-commits at lists.llvm.org
Thu Aug 31 08:41:22 PDT 2023
Author: Victor Kingi
Date: 2023-08-31T15:41:16Z
New Revision: 12da8ef0e318cf1e05c1380de7b98bc5cfa51f42
URL: https://github.com/llvm/llvm-project/commit/12da8ef0e318cf1e05c1380de7b98bc5cfa51f42
DIFF: https://github.com/llvm/llvm-project/commit/12da8ef0e318cf1e05c1380de7b98bc5cfa51f42.diff
LOG: [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
Added:
Modified:
flang/include/flang/Frontend/TextDiagnosticPrinter.h
flang/lib/Frontend/CompilerInvocation.cpp
flang/lib/Frontend/FrontendActions.cpp
flang/lib/Frontend/TextDiagnosticPrinter.cpp
flang/test/Driver/optimization-remark-backend.f90
flang/test/Driver/optimization-remark-invalid.f90
flang/test/Driver/optimization-remark.f90
Removed:
################################################################################
diff --git a/flang/include/flang/Frontend/TextDiagnosticPrinter.h b/flang/include/flang/Frontend/TextDiagnosticPrinter.h
index 0e092a0a012e07..9c99a0c314351f 100644
--- a/flang/include/flang/Frontend/TextDiagnosticPrinter.h
+++ b/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
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 56218cbadd444d..81bf89b1a44e5a 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/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();
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index e61178bb83ddc5..3ca667e64ec25c 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/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.
diff --git a/flang/lib/Frontend/TextDiagnosticPrinter.cpp b/flang/lib/Frontend/TextDiagnosticPrinter.cpp
index 7ae19645e40a40..1e6414f38648e0 100644
--- a/flang/lib/Frontend/TextDiagnosticPrinter.cpp
+++ b/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();
}
diff --git a/flang/test/Driver/optimization-remark-backend.f90 b/flang/test/Driver/optimization-remark-backend.f90
index a38b024b7b8d20..5d455f09d31523 100644
--- a/flang/test/Driver/optimization-remark-backend.f90
+++ b/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
diff --git a/flang/test/Driver/optimization-remark-invalid.f90 b/flang/test/Driver/optimization-remark-invalid.f90
index a57be4ec3e729d..46bc5c0f3e83f8 100644
--- a/flang/test/Driver/optimization-remark-invalid.f90
+++ b/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
diff --git a/flang/test/Driver/optimization-remark.f90 b/flang/test/Driver/optimization-remark.f90
index eb24a91d8db51b..145121a4574f6a 100644
--- a/flang/test/Driver/optimization-remark.f90
+++ b/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
More information about the flang-commits
mailing list