[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