[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi abandoned this revision.
victorkingi added a comment.

Replaced by a series of patches already merged on github, starting from 
https://reviews.llvm.org/D157410


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi added a comment.

In D156320#4594584 , @awarzynski 
wrote:

> Victor, this is proving quite tricky to review. There's already been a lot of 
> updates and many of them are summarized as either "code refactor" or 
> "clean-up". Please reduce traffic/noise and use more descriptive summaries.
>
> Also, rather than adding new features in this already large change (I am 
> referring to e.g. `DK_MachineOptimizationRemarkMissed`), please try to 
> identify ways to split this patch further. Here are some suggestions (I've 
> also made comments inline):
>
> 1. In the first iteration (like you effectively do now), focus on  
> OPT_R_Joined 
> 
>  options (e.g. `-Rpass`, `Rpass-analysis`, `-Rpass-missed`). Focus on basic 
> functionality that demonstrates that correct information is returned from the 
> backend. No need to fine tune the remarks with e.g. full file path or 
> relevant remark option.
> 2. Next, add support for -Rpass=/-Rpass-missed=/-Rpass-analysis= 
> .
>  That's when e.g. `llvm::opt::OptSpecifier optEq` in 
> `parseOptimizationRemark` would be needed (but not in Step 1).
> 3. Fine tune how the report is printed (e.g. improve file name by adding full 
> path, add valid remark option at the end etc).
> 4. Add support for machine optimisations, e.g. 
> `DK_MachineOptimizationRemarkMissed`.
>
> This is easily 4 patches ;-)

Hi Andrzej, splitting into 4 patches seems like a good idea. Here's the first 
patch that only handles the backend implementation 
https://reviews.llvm.org/D158174


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Victor, this is proving quite tricky to review. There's already been a lot of 
updates and many of them are summarized as either "code refactor" or 
"clean-up". Please reduce traffic/noise and use more descriptive summaries.

Also, rather than adding new features in this already large change (I am 
referring to e.g. `DK_MachineOptimizationRemarkMissed`), please try to identify 
ways to split this patch further. Here are some suggestions (I've also made 
comments inline):

1. In the first iteration (like you effectively do now), focus on  OPT_R_Joined 

 options (e.g. `-Rpass`, `Rpass-analysis`, `-Rpass-missed`). Focus on basic 
functionality that demonstrates that correct information is returned from the 
backend. No need to fine tune the remarks with e.g. full file path or relevant 
remark option.
2. Next, add support for -Rpass=/-Rpass-missed=/-Rpass-analysis= 
.
 That's when e.g. `llvm::opt::OptSpecifier optEq` in `parseOptimizationRemark` 
would be needed (but not in Step 1).
3. Fine tune how the report is printed (e.g. improve file name by adding full 
path, add valid remark option at the end etc).
4. Add support for machine optimisations, e.g. 
`DK_MachineOptimizationRemarkMissed`.

This is easily 4 patches ;-)




Comment at: flang/lib/Frontend/CompilerInvocation.cpp:158
+parseOptimizationRemark(clang::DiagnosticsEngine &diags,
+llvm::opt::ArgList &args, llvm::opt::OptSpecifier 
optEq,
+llvm::StringRef name) {

`optEq` is not used, but it should be.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:244-246
+  // Get -Rpass option regex. If empty, "".*"" is used. From all successful
+  // optimization passes applied, the regex will return only pass names that
+  // match it.

This is for `-Rpass=`, which is not tested. And the comment is inaccurate.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:250-252
+  // Get -Rpass-missed option regex. If empty, "".*"" is used. From all
+  // optimization passes that failed to be applied, the regex will return only
+  // pass names that match it.

This is for `-Rpass-missed=`, which is not tested. And the comment is 
inaccurate.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:256-257
+
+  // Specify which passes, with additional information,
+  // should be reported using a regex.
+  opts.OptimizationRemarkAnalysis = parseOptimizationRemark(

This is for `-Rpass-analysis=`, which is not tested. And the comment is 
inaccurate.



Comment at: flang/lib/Frontend/FrontendActions.cpp:1032-1043
+case llvm::DK_MachineOptimizationRemark:
+  optimizationRemarkHandler(
+  llvm::cast(di));
+  break;
+case llvm::DK_MachineOptimizationRemarkMissed:
+  optimizationRemarkHandler(
+  llvm::cast(di));

victorkingi wrote:
> This cases should handle back-end passes as the previous cases only handled 
> middle-end
Please move this to a dedicated patch with a test for each of these cases.



Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:19
 #include "flang/Frontend/TextDiagnostic.h"
+#include "string"
 #include "clang/Basic/DiagnosticOptions.h"

https://llvm.org/docs/CodingStandards.html#include-style
 
>   1.  Main Module Header
>   2.  Local/Private Headers
>   3.  LLVM project/subproject headers (clang/..., lldb/..., llvm/..., etc)
>   4.  System #includes




Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:39-54
+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

Move to a dedicated patch.



Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:76-105
+  // split incoming string to get the absolute path and filename in the
+  // case we are receiving optimization remarks from BackendRemarkConsumer
+  std::string diagMsg = std::string(diagMessageStream.str());
+  std::string delimiter = ";;";
+
+  size_t pos = 0;
+  llvm::SmallVector tokens;

Move to a dedicated patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing lis

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[Thorsten via Phabricator via cfe-commits]

tschuett added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi added inline comments.



Comment at: flang/lib/Frontend/FrontendActions.cpp:1032-1043
+case llvm::DK_MachineOptimizationRemark:
+  optimizationRemarkHandler(
+  llvm::cast(di));
+  break;
+case llvm::DK_MachineOptimizationRemarkMissed:
+  optimizationRemarkHandler(
+  llvm::cast(di));

This cases should handle back-end passes as the previous cases only handled 
middle-end


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 550800.
victorkingi added a comment.

added support for backend remarks


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,58 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
+! RUN: %flang %s -O1 -Rno-pass 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! Check a backend pass can be generated
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=BACKEND
+
+
+! CHECK: optimization-remark.f90:54:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:49:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:49:5: remark: loop not vectorized: instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+! BACKEND: optimization-remark.f90:1:0: remark: 25 instructions in function [-Rpass-analysis=asm-printer]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -22,6 +22,7 @@
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/Pass/PassManager.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
@@ -100,6 +101,54 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+// Remarks are ignored by default in Diagnostic.td, hence, we have to
+// enable them here before execution. Clang follows same idea using
+// ProcessWarningOptions in Warnings.cpp
+// This function is also responsible for emitting early warnings for
+// invalid -R options.
+static void
+updateDiagEngineForOptRemarks(clang::DiagnosticsEngine &diagsEng,
+  const clang::DiagnosticOptions &opts) {
+  llvm::SmallVector diags;
+  cons

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi added inline comments.



Comment at: flang/test/Driver/optimization-remark.f90:7
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s 
--check-prefix=CHECK-NO-REMARKS
+

awarzynski wrote:
> How about something like this:
> ```
> ! RUN: %flang_fc1 %s -O1 -Runsupported_remark_opt -Rpass -emit-llvm -o - 2>&1 
> | FileCheck %s
> ```
We are testing here if `-Rno-pass`  is provided after `-Rpass`, nothing should 
be printed.

Unsupported value is tested at 

> ! Check "unknown remark option" warning with suggestion
> ! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s 
> --check-prefix=CHECK-WARN-SUGGEST



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227
 
+  // Specifies, using a regex, which successful optimization passes done,
+  // to include in the final optimization record file generated. If not 
provided

victorkingi wrote:
> tschuett wrote:
> > awarzynski wrote:
> > > Do you know whether that only includes middle-end, or also back-end 
> > > passes?
> > I use -Rpass-missed='gisel*'  for GlobalIsel aka backend. I am interested 
> > in doing that exercise with Flang.
> Includes both middle and backend
No. There are no middle-end passes that match that pattern.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi added inline comments.



Comment at: flang/include/flang/Frontend/CodeGenOptions.h:76-81
+RK_Missing,// Remark argument not present on the command line.
+RK_Enabled,// Remark enabled via '-Rgroup'.
+RK_EnabledEverything,  // Remark enabled via '-Reverything'.
+RK_Disabled,   // Remark disabled via '-Rno-group'.
+RK_DisabledEverything, // Remark disabled via '-Rno-everything'.
+RK_WithPattern,// Remark pattern specified via '-Rgroup=regexp'.

awarzynski wrote:
> I only see `RK_Enabled` and `RK_Disabled` being used, though I don't see 
> `-Rgroup` nor `-Rno-group` being tested 🤔 .
`-Rgroup` represents `-Rpass`, `-Rpass-missed` and `-Rpass-analysis`. Same 
applies to the `no` variation



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227
 
+  // Specifies, using a regex, which successful optimization passes done,
+  // to include in the final optimization record file generated. If not 
provided

tschuett wrote:
> awarzynski wrote:
> > Do you know whether that only includes middle-end, or also back-end passes?
> I use -Rpass-missed='gisel*'  for GlobalIsel aka backend. I am interested in 
> doing that exercise with Flang.
Includes both middle and backend



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:240
+  // OptimizationRemark, OptimizationRemarkMissed and 
OptimizationRemarkAnalysis
+  // contain regex values which are used in optimizationRemarkHandler in
+  // FrontendActions.cpp to determine which remarks generated should be 
outputed

awarzynski wrote:
> `optimizationRemarkHandler` is a member method of `DiagnosticHandler`, that 
> you specialise in FrontendActions.cpp, right?
No, it's just a member method of BackendRemarkConsumer



Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:23
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "llvm/ADT/SmallString.h"

awarzynski wrote:
> Is this needed?
No it's not, I've removed it


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 550725.
victorkingi added a comment.

code refactoring


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,52 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
+! RUN: %flang %s -O1 -Rno-pass 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:48:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:43:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:43:5: remark: loop not vectorized: instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -22,6 +22,7 @@
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/Pass/PassManager.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
@@ -100,6 +101,54 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+// Remarks are ignored by default in Diagnostic.td, hence, we have to
+// enable them here before execution. Clang follows same idea using
+// ProcessWarningOptions in Warnings.cpp
+// This function is also responsible for emitting early warnings for
+// invalid -R options.
+static void
+updateDiagEngineForOptRemarks(clang::DiagnosticsEngine &diagsEng,
+  const clang::DiagnosticOptions &opts) {
+  llvm::SmallVector diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diagsEng.getDiagnosticIDs();
+
+  for (unsigned i = 0; i < opts.Remarks.size(); i++) {
+llvm::StringRef remarkOpt = opts.Remarks[i];
+const auto flavor = clang::diag::Flavor::Remark;
+
+// Check t

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 550719.
victorkingi marked 9 inline comments as done.
victorkingi added a comment.

code refactoring


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,52 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
+! RUN: %flang %s -O1 -Rno-pass 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:48:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:43:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:43:5: remark: loop not vectorized: instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -22,6 +22,7 @@
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/Pass/PassManager.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
@@ -100,6 +101,54 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+// Remarks are ignored by default in Diagnostic.td, hence, we have to
+// enable them here before execution. Clang follows same idea using
+// ProcessWarningOptions in Warnings.cpp
+// This function is also responsible for emitting early warnings for
+// invalid -R options.
+static void
+updateDiagEngineForOptRemarks(clang::DiagnosticsEngine &diagsEng,
+  const clang::DiagnosticOptions &opts) {
+  llvm::SmallVector diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diagsEng.getDiagnosticIDs();
+
+  for (unsigned i = 0; i < opts.Remarks.size(); i++) {
+llvm::StringRef remarkOpt = opts.Remarks[i];
+const auto flavor = 

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[Thorsten via Phabricator via cfe-commits]

tschuett added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227
 
+  // Specifies, using a regex, which successful optimization passes done,
+  // to include in the final optimization record file generated. If not 
provided

awarzynski wrote:
> Do you know whether that only includes middle-end, or also back-end passes?
I use -Rpass-missed='gisel*'  for GlobalIsel aka backend. I am interested in 
doing that exercise with Flang.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

hey @victorkingi , I am still unsure about parsing these remarks options in two 
places:

- CompilerInvocation.cpp
- ExecuteCompilerInvocation.cpp

I think that it is important to clarify the relations between the two. In 
particular, it's normally the job of CompilerInvocaiton to make sure that e.g. 
`-Rpass -Rno-pass -Rpass` == `-Rpass` (so that `DiagnosticOptions::Remarks` 
only contains `-Rpass`). This might be tricky in practice if we want to support 
Regex, but would be good to document when e.g. populating 
`DiagnosticOptions::Remarks`.

I am also under the impression that extra complexity comes from the fact that 
this patch strives to support `-R` on top of 
`-R{no}pass`, `-R{no}pass-missed`, `-R{no}pass-analysis`. I also see some code 
left to support regex versions of the flags. Can you clean that up?




Comment at: flang/include/flang/Frontend/CodeGenOptions.h:76-81
+RK_Missing,// Remark argument not present on the command line.
+RK_Enabled,// Remark enabled via '-Rgroup'.
+RK_EnabledEverything,  // Remark enabled via '-Reverything'.
+RK_Disabled,   // Remark disabled via '-Rno-group'.
+RK_DisabledEverything, // Remark disabled via '-Rno-everything'.
+RK_WithPattern,// Remark pattern specified via '-Rgroup=regexp'.

I only see `RK_Enabled` and `RK_Disabled` being used, though I don't see 
`-Rgroup` nor `-Rno-group` being tested 🤔 .



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227
 
+  // Specifies, using a regex, which successful optimization passes done,
+  // to include in the final optimization record file generated. If not 
provided

Do you know whether that only includes middle-end, or also back-end passes?



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:240
+  // OptimizationRemark, OptimizationRemarkMissed and 
OptimizationRemarkAnalysis
+  // contain regex values which are used in optimizationRemarkHandler in
+  // FrontendActions.cpp to determine which remarks generated should be 
outputed

`optimizationRemarkHandler` is a member method of `DiagnosticHandler`, that you 
specialise in FrontendActions.cpp, right?



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:1034-1037
+  // Add the remark option requested i.e. pass, pass-missed or pass-analysis.
+  // This will be used later during processing warnings and remarks to give
+  // messages specific to a remark argument. That happens in
+  // processWarningOptions in ExecuteCompilerInvocation.cpp

How about:
```
Preserve all the remark options requested, e.g. -Rpass, -Rpass-missed or 
-Rpass-analysis. This will be used later when processing and outputting the 
remarks generated by LLVM in  ExecuteCompilerInvocation.cpp.
```



Comment at: flang/lib/Frontend/FrontendActions.cpp:976-1011
+  void
+  optimizationRemarkHandler(const llvm::DiagnosticInfoOptimizationBase &d) {
+if (d.isPassed()) {
+  // Optimization remarks are active only if the -Rpass flag has a regular
+  // expression that matches the name of the pass name in \p d.
+  if (codeGenOpts.OptimizationRemark.patternMatches(d.getPassName()))
+emitOptimizationMessage(

victorkingi wrote:
> awarzynski wrote:
> > 
> The if statement still needs to return if the pattern doesn't match, should I 
> leave it the way it is?
Sorry, my bad, I missed that. Yeah, then leave it as is, but could you replace 
`const llvm::DiagnosticInfoOptimizationBase &d` with something with more 
descriptive name? (I am referring to `d`)



Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:18
 #include "flang/Frontend/TextDiagnosticPrinter.h"
+#include "filesystem"
 #include "flang/Frontend/TextDiagnostic.h"

WOuld you be able to use 
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/Path.h 
instead?



Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:21
+#include "string"
+#include "vector"
 #include "clang/Basic/DiagnosticOptions.h"

Would you be able to use 
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/SmallVector.h
 instead?



Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:23
 #include "clang/Basic/DiagnosticOptions.h"
+#include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "llvm/ADT/SmallString.h"

Is this needed?



Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:122-146
+static void processRemarkOptions(clang::DiagnosticsEngine &diags,
+ const clang::DiagnosticOptions &opts,
+ bool reportDiags = true) {
+  llvm::SmallVector _diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diags.getDiagnosticIDs();
+

I am suggest

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 550345.
victorkingi added a comment.

rebasing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,52 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
+! RUN: %flang %s -O1 -Rno-pass 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:48:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:43:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:43:5: remark: loop not vectorized: instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -22,6 +22,7 @@
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/Pass/PassManager.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
@@ -100,6 +101,50 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+// Remarks are ignored by default in Diagnostic.td, hence, we have to
+// enable them here before execution. Clang follows same idea using
+// ProcessWarningOptions in Warnings.cpp
+// This function is also responsible for emitting early warnings for
+// invalid -R options.
+static void processRemarkOptions(clang::DiagnosticsEngine &diags,
+ const clang::DiagnosticOptions &opts,
+ bool reportDiags = true) {
+  llvm::SmallVector _diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diags.getDiagnosticIDs();
+
+  for (unsigned i = 0; i < opts.Remarks.size(); i++) {
+llvm::StringRef opt = opts.Remarks[i];
+const auto flavor = clang::diag::

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 550320.
victorkingi added a comment.

rebasing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,52 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
+! RUN: %flang %s -O1 -Rno-pass 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:48:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:43:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:43:5: remark: loop not vectorized: instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -22,6 +22,7 @@
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/Pass/PassManager.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
@@ -100,6 +101,50 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+// Remarks are ignored by default in Diagnostic.td, hence, we have to
+// enable them here before execution. Clang follows same idea using
+// ProcessWarningOptions in Warnings.cpp
+// This function is also responsible for emitting early warnings for
+// invalid -R options.
+static void processRemarkOptions(clang::DiagnosticsEngine &diags,
+ const clang::DiagnosticOptions &opts,
+ bool reportDiags = true) {
+  llvm::SmallVector _diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diags.getDiagnosticIDs();
+
+  for (unsigned i = 0; i < opts.Remarks.size(); i++) {
+llvm::StringRef opt = opts.Remarks[i];
+const auto flavor = clang::diag::

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi added inline comments.



Comment at: flang/lib/Frontend/FrontendActions.cpp:976-1011
+  void
+  optimizationRemarkHandler(const llvm::DiagnosticInfoOptimizationBase &d) {
+if (d.isPassed()) {
+  // Optimization remarks are active only if the -Rpass flag has a regular
+  // expression that matches the name of the pass name in \p d.
+  if (codeGenOpts.OptimizationRemark.patternMatches(d.getPassName()))
+emitOptimizationMessage(

awarzynski wrote:
> 
The if statement still needs to return if the pattern doesn't match, should I 
leave it the way it is?



Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:30
 #include "llvm/Support/CommandLine.h"
+#include 
 

awarzynski wrote:
> No longer needed, right? Also, please use the the format consistent with the 
> other `#include`s.
It's needed in the case of `clang::diag::warn_unknown_diag_option` in 
`emitUnknownDiagWarning` function.



Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:117
+
+void processWarningOptions(clang::DiagnosticsEngine &diags,
+   const clang::DiagnosticOptions &opts,

awarzynski wrote:
> awarzynski wrote:
> > ?
> I find this method quite confusing.
> 
> 1. It doesn't process warning options - it processes remarks options (so the 
> name is inaccurate).
> 2. It deals with `-Rpass -Rno-pass` cases (i.e. postive + negative flag), but 
> that's normally done in CompilerInvocation when parsing other options. See 
> e.g. 
> https://github.com/llvm/llvm-project/blob/c52d9509d40d3048914b144618232213e6076e05/flang/lib/Frontend/CompilerInvocation.cpp#L161-L163.
>  Why not use that logic instead?
> 3. It's been extracted from Clang - it would be very helpful to note that and 
> highlight the difference between this method and similar method in Clang.
> 4. You can safely trim it to only deal with Remarks (so e.g. update `const 
> clang::DiagnosticOptions &opts,` in the signature)
> 
> Now, I am not requesting any major refactor here - I appreciate that e.g. 
> these remark flags are defined a bit differently to other flags. But this 
> method can be simplified and should be documented.
I found it difficult including it in `CompilerInvocation.cpp` 
`parseCodeGenArgs` function since we are updating the DiagnosticsEngine object 
belonging to CompilerInstance not the other. We would have to expose the 
`DiagnosticsEngine` in `CompilerInvocation` class to do this. Would there be 
another way?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 550288.
victorkingi added a comment.

code refactoring


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,52 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
+! RUN: %flang %s -O1 -Rno-pass 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:48:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:43:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:43:5: remark: loop not vectorized: instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -22,6 +22,7 @@
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/Pass/PassManager.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
@@ -100,6 +101,50 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+// Remarks are ignored by default in Diagnostic.td, hence, we have to
+// enable them here before execution. Clang follows same idea using
+// ProcessWarningOptions in Warnings.cpp
+// This function is also responsible for emitting early warnings for
+// invalid -R options.
+static void processRemarkOptions(clang::DiagnosticsEngine &diags,
+ const clang::DiagnosticOptions &opts,
+ bool reportDiags = true) {
+  llvm::SmallVector _diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diags.getDiagnosticIDs();
+
+  for (unsigned i = 0; i < opts.Remarks.size(); i++) {
+llvm::StringRef opt = opts.Remarks[i];
+const auto flavor = clang

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 550280.
victorkingi marked 4 inline comments as done.
victorkingi added a comment.

code refactoring


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,52 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
+! RUN: %flang %s -O1 -Rno-pass 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:48:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:43:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:43:5: remark: loop not vectorized: instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -22,6 +22,7 @@
 #include "mlir/IR/AsmState.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/Pass/PassManager.h"
+#include "clang/Basic/DiagnosticFrontend.h"
 #include "clang/Driver/Options.h"
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
@@ -100,6 +101,50 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+// Remarks are ignored by default in Diagnostic.td, hence, we have to
+// enable them here before execution. Clang follows same idea using
+// ProcessWarningOptions in Warnings.cpp
+// This function is also responsible for emitting early warnings for
+// invalid -R options.
+static void processRemarkOptions(clang::DiagnosticsEngine &diags,
+ const clang::DiagnosticOptions &opts,
+ bool reportDiags = true) {
+  llvm::SmallVector _diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diags.getDiagnosticIDs();
+
+  for (unsigned i = 0, e = opts.Remarks.size(); i != e; ++i) {
+llvm::StringRef

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 550232.
victorkingi marked 5 inline comments as done.
victorkingi added a comment.

code refactoring


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,52 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
+! RUN: %flang %s -O1 -Rno-pass 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:48:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:43:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:43:5: remark: loop not vectorized: instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Option/Option.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
+#include 
 
 namespace Fortran::frontend {
 
@@ -100,6 +101,61 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+static void processWarningOptions(clang::DiagnosticsEngine &diags,
+  const clang::DiagnosticOptions &opts,
+  bool reportDiags = true) {
+
+  llvm::SmallVector _diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diags.getDiagnosticIDs();
+  // We parse the warning options twice.  The first pass sets diagnostic state,
+  // while the second pass reports warnings/errors.  This has the effect that
+  // we follow the more canonical "last option wins" paradigm when there are
+  // conflicting options.
+  for (unsigned report = 0, reportEnd = 2; report != reportEnd; ++report) {
+bool setDiagnostic = (report == 0);
+
+// If we've set the diagnostic state and are not reporting 

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Some high level comments:

- The logic in `parseCodeGenArgs` in CompilerInvocation.cpp is a bit complex 
and quite specialized - could you move it to a dedicated method?
- In a fair few places this patch make references to "diagnostic flags" or 
"diagnostic options". That's borrowed from Clang where there's one code path 
for `-W` and `-R` flags. Note that in Clang the logic for `-W` 
is vastly more complex. This is completely absent in Flang. So, everything 
that's added here is added specifically for "remarks" and it would be helpful 
to be explicit about that.  For example, `processWarningOptions` is misleading. 
`processRemakrsOptions` would be more accurate.

Thank you :)




Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227
 
+  // Specifies what passes to include. If not provided
+  // -fsave-optimization-record will include all passes.

> // Specifies what passes to include.

Could you be more specific, what passes are you referring to? Included where?



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:242
+
+  // Specify which missed passes should be reported using a regex.
+  opts.OptimizationRemarkMissed = parseOptimizationRemark(

>   // Specify which missed passes should be reported using a regex.

Reported where?



Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:37
+// Print any diagnostic option information to a raw_ostream.
+static void printDiagnosticOptions(llvm::raw_ostream &os,
+   clang::DiagnosticsEngine::Level level,

victorkingi wrote:
> awarzynski wrote:
> > Why is this method needed and can it be tested?
> This method prints out the pass that was done e.g. [-Rpass=inline ], 
> [-Rpass=loop-delete] next to the optimization message.
> It is tested by the full remark message emitted test in 
> flang/test/Driver/optimization-remark.f90
Flang has very _very_ limited support for warning flags. So this is going to be 
used specifically for Remarks. Please update and document accordingly.



Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:30
 #include "llvm/Support/CommandLine.h"
+#include 
 

No longer needed, right? Also, please use the the format consistent with the 
other `#include`s.



Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:117
+
+void processWarningOptions(clang::DiagnosticsEngine &diags,
+   const clang::DiagnosticOptions &opts,

awarzynski wrote:
> ?
I find this method quite confusing.

1. It doesn't process warning options - it processes remarks options (so the 
name is inaccurate).
2. It deals with `-Rpass -Rno-pass` cases (i.e. postive + negative flag), but 
that's normally done in CompilerInvocation when parsing other options. See e.g. 
https://github.com/llvm/llvm-project/blob/c52d9509d40d3048914b144618232213e6076e05/flang/lib/Frontend/CompilerInvocation.cpp#L161-L163.
 Why not use that logic instead?
3. It's been extracted from Clang - it would be very helpful to note that and 
highlight the difference between this method and similar method in Clang.
4. You can safely trim it to only deal with Remarks (so e.g. update `const 
clang::DiagnosticOptions &opts,` in the signature)

Now, I am not requesting any major refactor here - I appreciate that e.g. these 
remark flags are defined a bit differently to other flags. But this method can 
be simplified and should be documented.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Thanks for the updates - this is looking really good! A few more suggestions 
and then I'll scan the whole thing again (sorry, there's been quite a lot of 
code going back and forth).




Comment at: flang/lib/Frontend/CompilerInvocation.cpp:1024
 
+  // add the remark option requested i.e. pass, pass-missed or pass-analysis.
+  // This will be used later during processing warnings and remarks to give





Comment at: flang/lib/Frontend/FrontendActions.cpp:976-1011
+  void
+  optimizationRemarkHandler(const llvm::DiagnosticInfoOptimizationBase &d) {
+if (d.isPassed()) {
+  // Optimization remarks are active only if the -Rpass flag has a regular
+  // expression that matches the name of the pass name in \p d.
+  if (codeGenOpts.OptimizationRemark.patternMatches(d.getPassName()))
+emitOptimizationMessage(





Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:117
+
+void processWarningOptions(clang::DiagnosticsEngine &diags,
+   const clang::DiagnosticOptions &opts,

?



Comment at: flang/test/Driver/optimization-remark.f90:53
+end program forttest
\ No newline at end of file


FIXME


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi marked 6 inline comments as done.
victorkingi added inline comments.



Comment at: flang/lib/Frontend/FrontendActions.cpp:927
 
+class StandaloneBackendConsumer : public llvm::DiagnosticHandler {
+

awarzynski wrote:
> Why `StandaloneBackendConsumer`? Isn't this specifically for remarks? Also, 
> could you document this class a bit?
I've added a comment, does this give a good explanation of what it does?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi added inline comments.



Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:169-171
+  clang::ProcessWarningOptions(flang->getDiagnostics(),
+   flang->getDiagnosticOpts());
+

victorkingi wrote:
> awarzynski wrote:
> > Is this calling 
> > https://github.com/llvm/llvm-project/blob/c52d9509d40d3048914b144618232213e6076e05/clang/include/clang/Basic/Diagnostic.h#L1840-L1842?
> >  That's part of the `clangBasic` library. The overall goal in the driver is 
> > to reduce the dependency on Clang and this would be a step in the opposite 
> > direction. IIUC, we only need a small subset of that function, right?
> Yes, we only need a small subset. I'll create a static function in this file 
> to avoid the dependence
> Yes, we only need a small subset. I'll create a static function in this file 
> to avoid the dependence

I mean normal function not static


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:156-158
+/// Parse a remark command line argument. It may be missing, disabled/enabled 
by
+/// '-R[no-]group' or specified with a regular expression by '-Rgroup=regexp'.
+/// On top of that, it can be disabled/enabled globally by '-R[no-]everything'.

awarzynski wrote:
> kiranchandramohan wrote:
> > awarzynski wrote:
> > > Could you give an example (and write a test ) for `-Rgroup=regexp`. Also, 
> > > @kiranchandramohan , is this form actually needed? I am thinking that 
> > > it's a complexity that could be avoided. It  could definitely simplify 
> > > this patch.
> > `Rpass=regex` is used, particularly `Rpass=.`. So this is required, but can 
> > be deferred to a separate patch to simplify this one.
> >  can be deferred to a separate patch to simplify this one
> 
> That would be a small win - the complexity comes from the fact that we can't 
> rely on TableGen to define all possible options. 
Removed need for Rpass=regex 



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:786
   parseShowColorsArgs(args, /*defaultDiagColor=*/false);
+  res.getDiagnosticOpts().ShowColors = res.getFrontendOpts().showColors;
 

awarzynski wrote:
> victorkingi wrote:
> > Apparently without forwarding the color option to the CompilerInvocation, 
> > flang doesn't print remark errors with color. Hence the need for this.
> > Also, a question, why do we have 2 instances of DiagnosticsEngine, one in 
> > CompilerInvocation and the other passed as an argument?
> > Apparently without forwarding the color option to the CompilerInvocation, 
> > flang doesn't print remark errors with color. Hence the need for this.
> 
> It is already "forwarded" here: 
> https://github.com/llvm/llvm-project/blob/3a100ea901ed79d6a06a5f018be2b4d3bbca51e8/flang/lib/Frontend/CompilerInvocation.cpp#L117-L122.
>  Could you explain why you need this change _here_?
> 
> > Also, a question, why do we have 2 instances of DiagnosticsEngine, one in 
> > CompilerInvocation and the other passed as an argument?
> 
> [[ 
> https://github.com/llvm/llvm-project/blob/3a100ea901ed79d6a06a5f018be2b4d3bbca51e8/flang/tools/flang-driver/fc1_main.cpp#L37-L40
>  | One  ]] is for the driver itself, to generate diagnostics related to 
> "driver errors" (e.g. option parsing errors). The [[ 
> https://github.com/llvm/llvm-project/blob/3a100ea901ed79d6a06a5f018be2b4d3bbca51e8/flang/include/flang/Frontend/CompilerInstance.h#L64-L65
>  | other ]] belongs to `CompilerInstance` rather than `CompilerInvocation` 
> and is used to report compilation errors (e.g. semantic or parsing errors). 
Thanks for the explanation, A bad regex error would be printed without color. 
But since we are getting rid of the regex option, might as well remove this.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:173-174
+if (!result.Regex->isValid(regexError)) {
+  diags.Report(clang::diag::err_drv_optimization_remark_pattern)
+  << regexError << a->getAsString(args);
+  return false;

awarzynski wrote:
> Could you add a test to exercise this diagnostic?
added the tests in `optimization-remark.f90` These are the `-Rno-pass`, 
`-Rno-pass-analysis` and `-Rno-pass-missed` tests.



Comment at: flang/lib/Frontend/FrontendActions.cpp:1007-1024
+  bool handleDiagnostics(const llvm::DiagnosticInfo &di) override {
+switch (di.getKind()) {
+case llvm::DK_OptimizationRemark:
+  optimizationRemarkHandler(llvm::cast(di));
+  break;
+case llvm::DK_OptimizationRemarkMissed:
+  
optimizationRemarkHandler(llvm::cast(di));

awarzynski wrote:
> Where is this method used?
`llvm/include/llvm/IR/DiagnosticHandler.h` specifies that this method needs to 
be overridden if one wants to setup custom diagnostic format reporting. 
`llvm/lib/IR/LLVMContext.cpp` then uses it for reporting in the diagnose 
function


```
void LLVMContext::diagnose(const DiagnosticInfo &DI) {
  if (auto *OptDiagBase = dyn_cast(&DI))
if (LLVMRemarkStreamer *RS = getLLVMRemarkStreamer())
  RS->emit(*OptDiagBase);

  // If there is a report handler, use it.
  if (pImpl->DiagHandler &&
  (!pImpl->RespectDiagnosticFilters || isDiagnosticEnabled(DI)) &&
  pImpl->DiagHandler->handleDiagnostics(DI))
return;
  .
  .
  .
```



Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:37
+// Print any diagnostic option information to a raw_ostream.
+static void printDiagnosticOptions(llvm::raw_ostream &os,
+   clang::DiagnosticsEngine::Level level,

awarzynski wrote:
> Why is this method needed and can it be tested?
This method prints out the pass that was done e.g. [-Rpass=inline ], 
[-Rpass=loop-delete] next to the optimization message.
It is tested by the full remark message emitted test in 
flang/test/

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 549887.
victorkingi added a comment.

added comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,52 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
+! RUN: %flang %s -O1 -Rno-pass 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:49:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:44:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:44:5: remark: loop not vectorized: instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
\ No newline at end of file
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Option/Option.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
+#include 
 
 namespace Fortran::frontend {
 
@@ -100,6 +101,61 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+void processWarningOptions(clang::DiagnosticsEngine &diags,
+   const clang::DiagnosticOptions &opts,
+   bool reportDiags = true) {
+
+  llvm::SmallVector _diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diags.getDiagnosticIDs();
+  // We parse the warning options twice.  The first pass sets diagnostic state,
+  // while the second pass reports warnings/errors.  This has the effect that
+  // we follow the more canonical "last option wins" paradigm when there are
+  // conflicting options.
+  for (unsigned report = 0, reportEnd = 2; report != reportEnd; ++report) {
+bool setDiagnostic = (report == 0);
+
+// If we've set the diagnostic state and are not reporting diagnostics then
+// we're done.
+

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 549882.
victorkingi added a comment.

added tests for "no" variants of Rpass


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,52 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
+! RUN: %flang %s -O1 -Rno-pass 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=CHECK-NO-REMARKS
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:49:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:44:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:44:5: remark: loop not vectorized: instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
\ No newline at end of file
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Option/Option.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
+#include 
 
 namespace Fortran::frontend {
 
@@ -100,6 +101,61 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+void processWarningOptions(clang::DiagnosticsEngine &diags,
+   const clang::DiagnosticOptions &opts,
+   bool reportDiags = true) {
+
+  llvm::SmallVector _diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diags.getDiagnosticIDs();
+  // We parse the warning options twice.  The first pass sets diagnostic state,
+  // while the second pass reports warnings/errors.  This has the effect that
+  // we follow the more canonical "last option wins" paradigm when there are
+  // conflicting options.
+  for (unsigned report = 0, reportEnd = 2; report != reportEnd; ++report) {
+bool setDiagnostic = (report == 0);
+
+// If we've set the diagnostic state and are not reporting diagnostics then

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 549879.
victorkingi marked 6 inline comments as done.
victorkingi added a comment.

code refactoring in reference to comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,47 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:43:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:38:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:40:9: remark: loop not vectorized: call instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
\ No newline at end of file
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -27,6 +27,7 @@
 #include "llvm/Option/Option.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
+#include 
 
 namespace Fortran::frontend {
 
@@ -100,6 +101,61 @@
   llvm_unreachable("Invalid program action!");
 }
 
+// Emit a warning and typo hint for unknown warning opts
+static void emitUnknownDiagWarning(clang::DiagnosticsEngine &diags,
+   clang::diag::Flavor flavor,
+   llvm::StringRef prefix,
+   llvm::StringRef opt) {
+  llvm::StringRef suggestion =
+  clang::DiagnosticIDs::getNearestOption(flavor, opt);
+  diags.Report(clang::diag::warn_unknown_diag_option)
+  << (flavor == clang::diag::Flavor::WarningOrError ? 0 : 1)
+  << (prefix.str() += std::string(opt)) << !suggestion.empty()
+  << (prefix.str() += std::string(suggestion));
+}
+
+void processWarningOptions(clang::DiagnosticsEngine &diags,
+   const clang::DiagnosticOptions &opts,
+   bool reportDiags = true) {
+
+  llvm::SmallVector _diags;
+  const llvm::IntrusiveRefCntPtr diagIDs =
+  diags.getDiagnosticIDs();
+  // We parse the warning options twice.  The first pass sets diagnostic state,
+  // while the second pass reports warnings/errors.  This has the effect that
+  // we follow the more canonical "last option wins" paradigm when there are
+  // conflicting options.
+  for (unsigned report = 0, reportEnd = 2; report != reportEnd; ++report) {
+bool setDiagnostic = (report == 0);
+
+// If we've set the diagnostic state and are not reporting diagnostics then
+// we're done.
+if (!setDiagnostic && !reportDiags)
+  break;
+
+for (unsigned i = 0, e = opts.Remarks.size(); i != e; ++i) {
+  llvm::StringRef opt = opts.Remarks[i];
+  const auto flavor = clang::diag::Flavor::Remark;
+
+  // Check to see if this opt starts with "no-", if so, this is a
+  // neg

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Thanks for trimming this, it's much easier to review! A few more suggestions, 
but nothing major.




Comment at: flang/lib/Frontend/CompilerInvocation.cpp:227-263
+  bool needLocTracking = false;
+
+  if (!opts.OptRecordFile.empty())
+needLocTracking = true;
 
   if (const llvm::opt::Arg *a =
+  args.getLastArg(clang::driver::options::OPT_opt_record_passes)) {

```lang=cpp
  // coment
  if (const llvm::opt::Arg *a =
  args.getLastArg(clang::driver::options::OPT_opt_record_passes))
opts.OptRecordPasses = a->getValue();

  // coment
  if (const llvm::opt::Arg *a =
  args.getLastArg(clang::driver::options::OPT_opt_record_format))
opts.OptRecordFormat = a->getValue();

  // coment
  opts.OptimizationRemark = parseOptimizationRemark(
  diags, args, clang::driver::options::OPT_Rpass_EQ, "pass");

  // coment
  opts.OptimizationRemarkMissed = parseOptimizationRemark(
  diags, args, clang::driver::options::OPT_Rpass_missed_EQ, "pass-missed");

  // coment
  opts.OptimizationRemarkAnalysis = parseOptimizationRemark(
  diags, args, clang::driver::options::OPT_Rpass_analysis_EQ,
  "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 
||
   !opts.OptRecordFormat.empty() ||
   opts.OptimizationRemark.hasValidPattern() ||
   opts.OptimizationRemarkMissed.hasValidPattern() ||
   opts.OptimizationRemarkAnalysis.hasValidPattern();

if (needLocTracking)
  opts.setDebugInfo(llvm::codegenoptions::LocTrackingOnly);
  }
```

I might have made typos when editing this, but hopefully the overall logic 
makes sense. Basically, I am suggesting that "option parsing" is separated from 
the logic for setting up the location tracking in the backend.



Comment at: flang/lib/Frontend/FrontendActions.cpp:978-1005
+if (d.isPassed()) {
+  // Optimization remarks are active only if the -Rpass flag has a regular
+  // expression that matches the name of the pass name in \p D.
+  if (codeGenOpts.OptimizationRemark.patternMatches(d.getPassName()))
+emitOptimizationMessage(
+d, clang::diag::remark_fe_backend_optimization_remark);
+

https://llvm.org/docs/CodingStandards.html#use-early-exits-and-continue-to-simplify-code

```lang=cpp
  void
  optimizationRemarkHandler(const llvm::DiagnosticInfoOptimizationBase &d) {
if (d.isPassed()) {
  // Optimization remarks are active only if the -Rpass flag has a regular
  // expression that matches the name of the pass name in \p D.
  if (codeGenOpts.OptimizationRemark.patternMatches(d.getPassName()))
emitOptimizationMessage(
d, clang::diag::remark_fe_backend_optimization_remark);
  return;
}

if (d.isMissed()) {
  // Missed optimization remarks are active only if the -Rpass-missed
  // flag has a regular expression that matches the name of the pass
  // name in \p D.
  if (codeGenOpts.OptimizationRemarkMissed.patternMatches(d.getPassName()))
emitOptimizationMessage(
d, clang::diag::remark_fe_backend_optimization_remark_missed);
  return;
}

assert(d.isAnalysis() && "Unknown remark type");

bool shouldAlwaysPrint = false;
if (auto *ora = llvm::dyn_cast(&d))
  shouldAlwaysPrint = ora->shouldAlwaysPrint();

if (shouldAlwaysPrint ||
codeGenOpts.OptimizationRemarkAnalysis.patternMatches(
d.getPassName()))
  emitOptimizationMessage(
  d, clang::diag::remark_fe_backend_optimization_remark_analysis);
  }
```



Comment at: flang/lib/Frontend/FrontendActions.cpp:1007-1024
+  bool handleDiagnostics(const llvm::DiagnosticInfo &di) override {
+switch (di.getKind()) {
+case llvm::DK_OptimizationRemark:
+  optimizationRemarkHandler(llvm::cast(di));
+  break;
+case llvm::DK_OptimizationRemarkMissed:
+  
optimizationRemarkHandler(llvm::cast(di));

Where is this method used?



Comment at: flang/lib/Frontend/FrontendActions.cpp:1136
 
+  BackendRemarkConsumer m(diags, codeGenOpts);
+

https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

> Variable names should be nouns (as they represent state). The name should be 
> camel case, and start with an upper case letter (e.g. Leader or Boats).



Comment at: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp:169-171
+  clang::ProcessWarningOptions(flang->getDiagnostics(),
+   flang->getDiagnosticOpts());
+
---

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 549367.
victorkingi marked an inline comment as done.
victorkingi added a comment.

addressing comments with code refactoring


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,47 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: optimization-remark.f90:43:5: remark: Loop deleted because it is invariant [-Rpass=loop-delete]
+! CHECK-REMARKS-MISSED: optimization-remark.f90:38:5: remark: loop not vectorized [-Rpass-missed=loop-vectorize]
+! CHECK-REMARKS-ANALYSIS: optimization-remark.f90:40:9: remark: loop not vectorized: call instruction cannot be vectorized [-Rpass-analysis=loop-vectorize]
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -166,6 +166,9 @@
   // Honor color diagnostics.
   flang->getDiagnosticOpts().ShowColors = flang->getFrontendOpts().showColors;
 
+  clang::ProcessWarningOptions(flang->getDiagnostics(),
+   flang->getDiagnosticOpts());
+
   // Create and execute the frontend action.
   std::unique_ptr act(createFrontendAction(*flang));
   if (!act)
Index: flang/lib/Frontend/TextDiagnosticPrinter.cpp
===
--- flang/lib/Frontend/TextDiagnosticPrinter.cpp
+++ flang/lib/Frontend/TextDiagnosticPrinter.cpp
@@ -20,6 +20,10 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
 
 using namespace Fortran::frontend;
 
@@ -29,6 +33,23 @@
 
 TextDiagnosticPrinter::~TextDiagnosticPrinter() {}
 
+// Print any diagnostic option information to a raw_ostream.
+static void printDiagnosticOptions(llvm::raw_ostream &os,
+   clang::DiagnosticsEngine::Level level,
+   const clang::Diagnostic &info,
+   const clang::DiagnosticOptions &diagOpts) {
+  llvm::StringRef opt =
+  clang::DiagnosticIDs::getWarningOptionForDiag(info.getID());
+  if (!opt.empty()) {
+os << " [" << (level == clang::DiagnosticsEngine::Remark ? "-R" : "-W")
+   << opt;
+llvm::StringRef optValue = info.getDiags()->getFlagValue();
+if (!optValue.empty())
+  os << "=" << optValue;
+os << ']';
+  }
+}
+
 void TextDiagnosticPrinter::HandleDiagnostic(
 clang::DiagnosticsEngine::Level level, const clang::Diagnostic &info) {
   // Default implementation (Warnings/errors count).
@@ -40,6 +61,7 @@
   info.FormatDiagnostic(outStr);
 
   llvm::raw_svector_ostream diagMessageStream(outStr);
+  printDiagnosticOptions(diagMessageStream, level, info, *diagOpts);
 
   if (!prefix.empty())
 os << prefix << ": ";
@@ -48,12 +70,46 @@
   a

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:156-158
+/// Parse a remark command line argument. It may be missing, disabled/enabled 
by
+/// '-R[no-]group' or specified with a regular expression by '-Rgroup=regexp'.
+/// On top of that, it can be disabled/enabled globally by '-R[no-]everything'.

kiranchandramohan wrote:
> awarzynski wrote:
> > Could you give an example (and write a test ) for `-Rgroup=regexp`. Also, 
> > @kiranchandramohan , is this form actually needed? I am thinking that it's 
> > a complexity that could be avoided. It  could definitely simplify this 
> > patch.
> `Rpass=regex` is used, particularly `Rpass=.`. So this is required, but can 
> be deferred to a separate patch to simplify this one.
>  can be deferred to a separate patch to simplify this one

That would be a small win - the complexity comes from the fact that we can't 
rely on TableGen to define all possible options. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[Kiran Chandramohan via Phabricator via cfe-commits]

kiranchandramohan added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:156-158
+/// Parse a remark command line argument. It may be missing, disabled/enabled 
by
+/// '-R[no-]group' or specified with a regular expression by '-Rgroup=regexp'.
+/// On top of that, it can be disabled/enabled globally by '-R[no-]everything'.

awarzynski wrote:
> Could you give an example (and write a test ) for `-Rgroup=regexp`. Also, 
> @kiranchandramohan , is this form actually needed? I am thinking that it's a 
> complexity that could be avoided. It  could definitely simplify this patch.
`Rpass=regex` is used, particularly `Rpass=.`. So this is required, but can be 
deferred to a separate patch to simplify this one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Thanks for the updates, more comments inline. Also:

> The remarks printed miss carets.

Why and can you share an example?

> The parseOptimizationRemark, addDiagnosticArgs and printDiagnosticOptions 
> functions created are identical to the ones used in Clang.

In which case we should seriously consider moving this code somewhere where it 
can be shared. If outside the scope of this change, please document in code 
that there's scope for re-use.




Comment at: flang/include/flang/Frontend/CodeGenOptions.h:72-118
+  enum class RemarkKind {
+RK_Missing,// Remark argument not present on the command line.
+RK_Enabled,// Remark enabled via '-Rgroup'.
+RK_EnabledEverything,  // Remark enabled via '-Reverything'.
+RK_Disabled,   // Remark disabled via '-Rno-group'.
+RK_DisabledEverything, // Remark disabled via '-Rno-everything'.
+RK_WithPattern,// Remark pattern specified via '-Rgroup=regexp'.

From what I can see, this has been borrowed almost verbatim from Clang: 
https://github.com/llvm/llvm-project/blob/3a100ea901ed79d6a06a5f018be2b4d3bbca51e8/clang/include/clang/Basic/CodeGenOptions.h#L331-L376.

This is fine (and common throughout the driver), but please document more. In 
particular:
* Highlight that ATM this code is identical that what Clang contains (and add a 
`TODO` to perhaps share with Clang at some point),
* Highlight that the list of options in Flang and Clang is _identical - it is 
really good that the interfaces in Clang and Flang are consistent. That's a 
good reason to re-use the logic from Clang.





Comment at: flang/lib/Frontend/CompilerInvocation.cpp:156-158
+/// Parse a remark command line argument. It may be missing, disabled/enabled 
by
+/// '-R[no-]group' or specified with a regular expression by '-Rgroup=regexp'.
+/// On top of that, it can be disabled/enabled globally by '-R[no-]everything'.

Could you give an example (and write a test ) for `-Rgroup=regexp`. Also, 
@kiranchandramohan , is this form actually needed? I am thinking that it's a 
complexity that could be avoided. It  could definitely simplify this patch.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:786
   parseShowColorsArgs(args, /*defaultDiagColor=*/false);
+  res.getDiagnosticOpts().ShowColors = res.getFrontendOpts().showColors;
 

victorkingi wrote:
> Apparently without forwarding the color option to the CompilerInvocation, 
> flang doesn't print remark errors with color. Hence the need for this.
> Also, a question, why do we have 2 instances of DiagnosticsEngine, one in 
> CompilerInvocation and the other passed as an argument?
> Apparently without forwarding the color option to the CompilerInvocation, 
> flang doesn't print remark errors with color. Hence the need for this.

It is already "forwarded" here: 
https://github.com/llvm/llvm-project/blob/3a100ea901ed79d6a06a5f018be2b4d3bbca51e8/flang/lib/Frontend/CompilerInvocation.cpp#L117-L122.
 Could you explain why you need this change _here_?

> Also, a question, why do we have 2 instances of DiagnosticsEngine, one in 
> CompilerInvocation and the other passed as an argument?

[[ 
https://github.com/llvm/llvm-project/blob/3a100ea901ed79d6a06a5f018be2b4d3bbca51e8/flang/tools/flang-driver/fc1_main.cpp#L37-L40
 | One  ]] is for the driver itself, to generate diagnostics related to "driver 
errors" (e.g. option parsing errors). The [[ 
https://github.com/llvm/llvm-project/blob/3a100ea901ed79d6a06a5f018be2b4d3bbca51e8/flang/include/flang/Frontend/CompilerInstance.h#L64-L65
 | other ]] belongs to `CompilerInstance` rather than `CompilerInvocation` and 
is used to report compilation errors (e.g. semantic or parsing errors). 



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:1070
   }
 
+  addDiagnosticArgs(args, clang::driver::options::OPT_R_Group,

Is this specifically for parsing remarks options? Please add a comment



Comment at: flang/lib/Frontend/FrontendActions.cpp:927
 
+class StandaloneBackendConsumer : public llvm::DiagnosticHandler {
+

Why `StandaloneBackendConsumer`? Isn't this specifically for remarks? Also, 
could you document this class a bit?



Comment at: flang/lib/Frontend/TextDiagnosticPrinter.cpp:37
+// Print any diagnostic option information to a raw_ostream.
+static void printDiagnosticOptions(llvm::raw_ostream &os,
+   clang::DiagnosticsEngine::Level level,

Why is this method needed and can it be tested?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 549000.
victorkingi added a comment.

Added warning tests in optimization-remark.f90


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,59 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -Rno-everything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -Rno-everything -Reverything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+
+! -Reverything implies -Rpass=.*.
+! RUN: %flang_fc1 %s -O1 -Reverything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+
+! -Rpass implies -Rpass=.*
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+
+! Check error on bad regex
+! RUN: not %flang %s -O1 -Rpass=[ 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ERROR
+
+! Check "unknown remark option" warning
+! RUN: %flang %s -O1 -R 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-WARN
+
+! Check "unknown remark option" warning with suggestion
+! RUN: %flang %s -O1 -Rpas 2>&1 | FileCheck %s --check-prefix=CHECK-WARN-SUGGEST
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass=loop-delete 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed=loop-vectorize 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis=loop-vectorize 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+! CHECK: remark: Loop deleted because it is invariant
+! CHECK-REMARKS-MISSED: remark: loop not vectorized
+! CHECK-REMARKS-ANALYSIS: remark: loop not vectorized: call instruction cannot be vectorized
+! CHECK-REMARKS: remark:
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-ERROR: error: in pattern '-Rpass=[': brackets ([ ]) not balanced
+! CHECK-REMARKS-WARN: warning: unknown remark option '-R' [-Wunknown-warning-option]
+! CHECK-WARN-SUGGEST: warning: unknown remark option '-Rpas'; did you mean '-Rpass'? [-Wunknown-warning-option]
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -165,6 +165,10 @@
 
   // Honor color diagnostics.
   flang->getDiagnosticOpts().ShowColors = flang->getFrontendOpts().showColors;
+  flang->getDiagnosticOpts().ShowOptionNames = 1;
+
+  clang::ProcessWarningOptions(flang->getDiagnostics(),
+   flang->getDiagnosticOpts());
 
   // Create and execute the frontend action.
   std::unique_ptr act(createFrontendAction(*flang));
Index: flang/lib/Frontend/TextDiagnosticPrinter.cpp
===
--- flang/lib/Frontend/TextDiagnosticPrinter.cpp
+++ flang/lib/Frontend/TextDiagnosticPrinter.cpp
@@ -20,6 +20,10 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
 
 using namespace Fortran::frontend;
 
@@ -29,6 +33,28 @@
 
 TextDiagnosticPrinter::~TextDiagnosticPrinter() {}
 
+// Print any diagnostic option information to a raw_ostream.
+static void printDiagnosticOptions(llvm::raw_ostream &os,
+   clang::DiagnosticsEngine::Level level,
+   const clang::Diagnostic &info,
+   const clang::DiagnosticOptions &diagOpts) {
+  bool started = false;
+  if (diagOpts.ShowOptionNames) {
+llvm::StringRef opt =
+clang::DiagnosticIDs::getWarningOptionForDiag(info.getID());
+i

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 548950.
victorkingi added a comment.

Removed false argument to ProcessWarningOption function to allow warning 
printing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,55 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -Rno-everything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -Rno-everything -Reverything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+
+! -Reverything implies -Rpass=.*.
+! RUN: %flang_fc1 %s -O1 -Reverything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+
+! -Rpass implies -Rpass=.*
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+
+! Check error on bad regex
+! RUN: not %flang %s -O1 -Rpass=[ 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ERROR
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass=loop-delete 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed=loop-vectorize 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis=loop-vectorize 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+
+! CHECK: remark: Loop deleted because it is invariant
+
+! CHECK-REMARKS-MISSED: remark: loop not vectorized
+! CHECK-REMARKS-ANALYSIS: remark: loop not vectorized: call instruction cannot be vectorized
+! CHECK-REMARKS: remark:
+
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-ERROR: error: in pattern '-Rpass=[': brackets ([ ]) not balanced
+
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -165,6 +165,10 @@
 
   // Honor color diagnostics.
   flang->getDiagnosticOpts().ShowColors = flang->getFrontendOpts().showColors;
+  flang->getDiagnosticOpts().ShowOptionNames = 1;
+
+  clang::ProcessWarningOptions(flang->getDiagnostics(),
+   flang->getDiagnosticOpts());
 
   // Create and execute the frontend action.
   std::unique_ptr act(createFrontendAction(*flang));
Index: flang/lib/Frontend/TextDiagnosticPrinter.cpp
===
--- flang/lib/Frontend/TextDiagnosticPrinter.cpp
+++ flang/lib/Frontend/TextDiagnosticPrinter.cpp
@@ -20,6 +20,10 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
 
 using namespace Fortran::frontend;
 
@@ -29,6 +33,28 @@
 
 TextDiagnosticPrinter::~TextDiagnosticPrinter() {}
 
+// Print any diagnostic option information to a raw_ostream.
+static void printDiagnosticOptions(llvm::raw_ostream &os,
+   clang::DiagnosticsEngine::Level level,
+   const clang::Diagnostic &info,
+   const clang::DiagnosticOptions &diagOpts) {
+  bool started = false;
+  if (diagOpts.ShowOptionNames) {
+llvm::StringRef opt =
+clang::DiagnosticIDs::getWarningOptionForDiag(info.getID());
+if (!opt.empty()) {
+  os << (started ? "," : " [")
+ << (level == clang::DiagnosticsEngine::Remark ? "-R" : "-W") << opt;
+  llvm::StringRef optValue = info.getDiags()->getFlagValue();
+  if (!optValue.empty())
+os << "=" << optValue;
+  started = true;
+}
+  }
+  if (started)
+os << ']';
+}
+
 void TextDiagnosticPrinter::HandleDiagnostic(
 clang::DiagnosticsEngine::Level

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 548941.
victorkingi added a comment.

rebasing


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,55 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -Rno-everything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -Rno-everything -Reverything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+
+! -Reverything implies -Rpass=.*.
+! RUN: %flang_fc1 %s -O1 -Reverything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+
+! -Rpass implies -Rpass=.*
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+
+! Check error on bad regex
+! RUN: not %flang %s -O1 -Rpass=[ 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ERROR
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass=loop-delete 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed=loop-vectorize 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis=loop-vectorize 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+
+! CHECK: remark: Loop deleted because it is invariant
+
+! CHECK-REMARKS-MISSED: remark: loop not vectorized
+! CHECK-REMARKS-ANALYSIS: remark: loop not vectorized: call instruction cannot be vectorized
+! CHECK-REMARKS: remark:
+
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-ERROR: error: in pattern '-Rpass=[': brackets ([ ]) not balanced
+
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -165,6 +165,10 @@
 
   // Honor color diagnostics.
   flang->getDiagnosticOpts().ShowColors = flang->getFrontendOpts().showColors;
+  flang->getDiagnosticOpts().ShowOptionNames = 1;
+
+  clang::ProcessWarningOptions(flang->getDiagnostics(),
+   flang->getDiagnosticOpts(), false);
 
   // Create and execute the frontend action.
   std::unique_ptr act(createFrontendAction(*flang));
Index: flang/lib/Frontend/TextDiagnosticPrinter.cpp
===
--- flang/lib/Frontend/TextDiagnosticPrinter.cpp
+++ flang/lib/Frontend/TextDiagnosticPrinter.cpp
@@ -20,6 +20,10 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
 
 using namespace Fortran::frontend;
 
@@ -29,6 +33,28 @@
 
 TextDiagnosticPrinter::~TextDiagnosticPrinter() {}
 
+// Print any diagnostic option information to a raw_ostream.
+static void printDiagnosticOptions(llvm::raw_ostream &os,
+   clang::DiagnosticsEngine::Level level,
+   const clang::Diagnostic &info,
+   const clang::DiagnosticOptions &diagOpts) {
+  bool started = false;
+  if (diagOpts.ShowOptionNames) {
+llvm::StringRef opt =
+clang::DiagnosticIDs::getWarningOptionForDiag(info.getID());
+if (!opt.empty()) {
+  os << (started ? "," : " [")
+ << (level == clang::DiagnosticsEngine::Remark ? "-R" : "-W") << opt;
+  llvm::StringRef optValue = info.getDiags()->getFlagValue();
+  if (!optValue.empty())
+os << "=" << optValue;
+  started = true;
+}
+  }
+  if (started)
+os << ']';
+}
+
 void TextDiagnosticPrinter::HandleDiagnostic(
 clang::DiagnosticsEngine::Level level, const clang::Diagnostic &info) {
   // Default implementati

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi marked an inline comment as done.
victorkingi added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:786
   parseShowColorsArgs(args, /*defaultDiagColor=*/false);
+  res.getDiagnosticOpts().ShowColors = res.getFrontendOpts().showColors;
 

Apparently without forwarding the color option to the CompilerInvocation, flang 
doesn't print remark errors with color. Hence the need for this.
Also, a question, why do we have 2 instances of DiagnosticsEngine, one in 
CompilerInvocation and the other passed as an argument?



Comment at: flang/test/Driver/optimization-remark.f90:17-18
+
+! Check error on bad regex
+! RUN: not %flang %s -O1 -Rpass=[ 2>&1 | FileCheck %s 
--check-prefix=CHECK-REMARKS-ERROR
+

Testing error produced given a bad regex


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[victorkingi via Phabricator via cfe-commits]

victorkingi updated this revision to Diff 548664.
victorkingi edited the summary of this revision.
victorkingi added a comment.

Added remark error test and color printing for remark errors


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

Files:
  flang/include/flang/Frontend/CodeGenOptions.h
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/TextDiagnosticPrinter.cpp
  flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
  flang/test/Driver/optimization-remark.f90

Index: flang/test/Driver/optimization-remark.f90
===
--- /dev/null
+++ flang/test/Driver/optimization-remark.f90
@@ -0,0 +1,55 @@
+! This file tests 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 that we can override -Rpass= with -Rno-pass.
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -Rno-everything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-NO-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass=loop-delete -Rno-everything -Reverything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+
+! -Reverything implies -Rpass=.*.
+! RUN: %flang_fc1 %s -O1 -Reverything -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+
+! -Rpass implies -Rpass=.*
+! RUN: %flang_fc1 %s -O1 -Rpass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS
+
+! Check error on bad regex
+! RUN: not %flang %s -O1 -Rpass=[ 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ERROR
+
+! Check full -Rpass message is emitted
+! RUN: %flang %s -O1 -Rpass=loop-delete 2>&1 | FileCheck %s
+
+! Check full -Rpass-missed message is emitted
+! RUN: %flang %s -O1 -Rpass-missed=loop-vectorize 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-MISSED
+
+! Check full -Rpass-analysis message is emitted
+! RUN: %flang %s -O1 -Rpass-analysis=loop-vectorize 2>&1 | FileCheck %s --check-prefix=CHECK-REMARKS-ANALYSIS
+
+
+! CHECK: remark: Loop deleted because it is invariant
+
+! CHECK-REMARKS-MISSED: remark: loop not vectorized
+! CHECK-REMARKS-ANALYSIS: remark: loop not vectorized: call instruction cannot be vectorized
+! CHECK-REMARKS: remark:
+
+! CHECK-NO-REMARKS-NOT: remark:
+
+! CHECK-REMARKS-ERROR: error: in pattern '-Rpass=[': brackets ([ ]) not balanced
+
+
+program forttest
+implicit none
+real, dimension(1:50) :: aR1
+integer :: n
+
+do n = 1,50
+aR1(n) = n * 1.34
+print *, "hello"
+end do
+
+do n = 1,50
+aR1(n) = n * 1.34
+end do
+
+end program forttest
Index: flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
===
--- flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
+++ flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -165,6 +165,10 @@
 
   // Honor color diagnostics.
   flang->getDiagnosticOpts().ShowColors = flang->getFrontendOpts().showColors;
+  flang->getDiagnosticOpts().ShowOptionNames = 1;
+
+  clang::ProcessWarningOptions(flang->getDiagnostics(),
+   flang->getDiagnosticOpts(), false);
 
   // Create and execute the frontend action.
   std::unique_ptr act(createFrontendAction(*flang));
Index: flang/lib/Frontend/TextDiagnosticPrinter.cpp
===
--- flang/lib/Frontend/TextDiagnosticPrinter.cpp
+++ flang/lib/Frontend/TextDiagnosticPrinter.cpp
@@ -20,6 +20,10 @@
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+#include 
 
 using namespace Fortran::frontend;
 
@@ -29,6 +33,28 @@
 
 TextDiagnosticPrinter::~TextDiagnosticPrinter() {}
 
+// Print any diagnostic option information to a raw_ostream.
+static void printDiagnosticOptions(llvm::raw_ostream &os,
+   clang::DiagnosticsEngine::Level level,
+   const clang::Diagnostic &info,
+   const clang::DiagnosticOptions &diagOpts) {
+  bool started = false;
+  if (diagOpts.ShowOptionNames) {
+llvm::StringRef opt =
+clang::DiagnosticIDs::getWarningOptionForDiag(info.getID());
+if (!opt.empty()) {
+  os << (started ? "," : " [")
+ << (level == clang::DiagnosticsEngine::Remark ? "-R" : "-W") << opt;
+  llvm::StringRef optValue = info.getDiags()->getFlagValue();
+  if (!optValue.empty())
+os << "=" << optValue;
+  started = true;
+}
+  }
+  if (started)
+os << ']';
+}
+
 void TextDiagnosticPrinter::HandleDiagnostic(
  

[PATCH] D156320: [Flang][Driver] Add support for Rpass and related options

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:173-174
+if (!result.Regex->isValid(regexError)) {
+  diags.Report(clang::diag::err_drv_optimization_remark_pattern)
+  << regexError << a->getAsString(args);
+  return false;

Could you add a test to exercise this diagnostic?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156320/new/

https://reviews.llvm.org/D156320

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits