[Flang][Driver] Add regex support for R_Group options

[victorkingi]

Summary

Add regex handling for all variations of OPT_R_Joined, i.e.
-Rpass, -Rpass-analysis, -Rpass-missed.

Depends on D158174. That patch implements backend support for
R_Group options.

[victorkingi]

flang/lib/Frontend/CompilerInvocation.cpp

764–766

For the bad regex error to be printed with colors, we would need to pass showColors from Frontend Options to Diagnostics Options much earlier on in CompilerInvocation::createFromArgs . Reason being regex errors from R family of flags don't get that far into the execution.

[awarzynski]

flang/lib/Frontend/CompilerInvocation.cpp

164

[nit] This is unrelated to this change, so just a nice-to-have

256

Just a nice-to-have

1033–1034

So, in -Rpass=<regex>you would only push -Rpass, right?

flang/test/Driver/optimization-remark.f90

18

At the moment this test does not differentiate between -Rpass=loop and -Rpass - in both cases the output is identical. Could you extend the test so that there's an actual difference?

[victorkingi]

Changed optimization-remark.f90 test code to generate more unique
missed and successful passes. -Rpass=loop and -Rpass should have
unique outputs hence, testable.
Minor comments added in CompilerInvocation.cpp

[victorkingi]

flang/lib/Frontend/CompilerInvocation.cpp

1033–1034

You will only push "pass". I've updated the comment to make it clearer.

[victorkingi]

Moved invalid regex test to optimization-remark-invalid.f90

[awarzynski]

Makes sense, a few minor suggestion. Also, could you rebase on top of main?

flang/test/Driver/optimization-remark.f90

15

[nit] IMHO, PASS-LOOP-ONLY would be more descriptive. Or even PASS-REGEX-LOOP-ONLY. "REGEX" refers to an implementation detail, but here we are more interested in the actual functionality.

35

Some comments explaining _what to expect_ would help. For example, with plain -Rpass we expect remarks related to 2 opportunities. Once we start filtering, this is reduced to 1, right?

39

Could you re-order the CHECK lines so that it matches the RUN lines? So:

! REMARKS:
! NO-REMARKS:

! PASS-REGEX-NOT:
! PASS-REGEX:

! MISSED-REGEX-NOT:
! MISSED-REGEX:

! ANALYSIS-REGEX-NOT:
! ANALYSIS-REGEX:

[victorkingi]

Reused same definition of the output in optimization-remark.f90 as well
as discarded the output generated.

Replaced '-c' flag references with '-S' to only handle pre-process and
compile steps in optimization-remark.f90.

flang/test/Driver/optimization-remark.f90

35

Yes

[awarzynski]

LGTM, thank you for addressing my comments!

flang/test/Driver/optimization-remark.f90

50–51

[victorkingi]

updated comment in optimization-remark.f90, attempt to fix failing test
case.

[victorkingi]

Attempt to fix failing optimization-remark.f90 test

[victorkingi]

[nit] optimization-remark.f90 line 80 'enddo' to 'end do'

[victorkingi]

no changes, rebasing

[victorkingi]

no changes, rebasing

flang/include/flang/Frontend/CodeGenOptions.h

public:
 
   /// The format used for serializing remarks (default: YAML)
   std::string OptRecordFormat;
 
   // The RemarkKind enum class and OptRemark struct are identical to what Clang
   // has
   // TODO: Share with clang instead of re-implementing here
   enum class RemarkKind {
-    RK_Missing,  // Remark argument not present on the command line.
-    RK_Enabled,  // Remark enabled via '-Rgroup', i.e. -Rpass, -Rpass-missed,
+    RK_Missing,     // Remark argument not present on the command line.
+    RK_Enabled,     // Remark enabled via '-Rgroup', i.e. -Rpass, -Rpass-missed,
                     // -Rpass-analysis
-    RK_Disabled, // Remark disabled via '-Rno-group', i.e. -Rno-pass,
+    RK_Disabled,    // Remark disabled via '-Rno-group', i.e. -Rno-pass,
                     // -Rno-pass-missed, -Rno-pass-analysis.
+    RK_WithPattern, // Remark pattern specified via '-Rgroup=regexp'.
   };
 
   /// Optimization remark with an optional regular expression pattern.
   struct OptRemark {
     RemarkKind Kind = RemarkKind::RK_Missing;
     std::string Pattern;
     std::shared_ptr<llvm::Regex> Regex;
 

flang/lib/Frontend/CompilerInvocation.cpp

if (llvm::opt::Arg *arg =
     }
   }
   return true;
 }
 
 // Generate an OptRemark object containing info on if the -Rgroup
 // specified is enabled or not.
 static CodeGenOptions::OptRemark
-parseOptimizationRemark(llvm::opt::ArgList &args,
+parseOptimizationRemark(clang::DiagnosticsEngine &diags,
+                        llvm::opt::ArgList &args, llvm::opt::OptSpecifier optEq,
                         llvm::StringRef remarkOptName) {
   assert((remarkOptName == "pass" || remarkOptName == "pass-missed" ||
           remarkOptName == "pass-analysis") &&
          "Unknown group name provided.");
   CodeGenOptions::OptRemark result;
 
   for (llvm::opt::Arg *a : args) {
     if (a->getOption().matches(clang::driver::options::OPT_R_Joined)) {
       llvm::StringRef value = a->getValue();
 
       if (value == remarkOptName) {
         result.Kind = CodeGenOptions::RemarkKind::RK_Enabled;
         // Enable everything
         result.Pattern = ".*";
         result.Regex = std::make_shared<llvm::Regex>(result.Pattern);
 
       } else if (value.split('-') ==
                  std::make_pair(llvm::StringRef("no"), remarkOptName)) {
         result.Kind = CodeGenOptions::RemarkKind::RK_Disabled;
         // Disable everything
         result.Pattern = "";
         result.Regex = nullptr;
       }
+    } else if (a->getOption().matches(optEq)) {
+      result.Kind = CodeGenOptions::RemarkKind::RK_WithPattern;
+      result.Pattern = a->getValue();
+      result.Regex = std::make_shared<llvm::Regex>(result.Pattern);
+      std::string regexError;
+
+      if (!result.Regex->isValid(regexError)) {
+        diags.Report(clang::diag::err_drv_optimization_remark_pattern)
+            << regexError << a->getAsString(args);
+        return CodeGenOptions::OptRemark();
+      }
     }
   }
   return result;
 }
 
 static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
                              llvm::opt::ArgList &args,
                              clang::DiagnosticsEngine &diags) {
static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
   // backend), to include in the final optimization record file generated. If
   // not provided -fsave-optimization-record will include all passes.
   if (const llvm::opt::Arg *a =
           args.getLastArg(clang::driver::options::OPT_opt_record_passes))
     opts.OptRecordPasses = a->getValue();
 
   // Create OptRemark that allows printing of all successful optimization
   // passes applied.
-  opts.OptimizationRemark = parseOptimizationRemark(args, "pass");
+  opts.OptimizationRemark = parseOptimizationRemark(
+      diags, args, clang::driver::options::OPT_Rpass_EQ, "pass");
 
   // Create OptRemark that allows all missed optimization passes to be printed.
-  opts.OptimizationRemarkMissed = parseOptimizationRemark(args, "pass-missed");
+  opts.OptimizationRemarkMissed = parseOptimizationRemark(
+      diags, args, clang::driver::options::OPT_Rpass_missed_EQ, "pass-missed");
 
   // Create OptRemark that allows all optimization decisions made by LLVM
   // to be printed.
-  opts.OptimizationRemarkAnalysis =
-      parseOptimizationRemark(args, "pass-analysis");
+  opts.OptimizationRemarkAnalysis = parseOptimizationRemark(
+      diags, args, clang::driver::options::OPT_Rpass_analysis_EQ,
+      "pass-analysis");
 
   if (auto *a = args.getLastArg(clang::driver::options::OPT_save_temps_EQ))
     opts.SaveTempsDir = a->getValue();
 
   // -mrelocation-model option.
   if (const llvm::opt::Arg *a =
           args.getLastArg(clang::driver::options::OPT_mrelocation_model)) {
     llvm::StringRef modelName = a->getValue();
for (const auto &wArg : wArgs) {
       }
     }
   }
 
   // Default to off for `flang-new -fc1`.
   res.getFrontendOpts().showColors =
       parseShowColorsArgs(args, /*defaultDiagColor=*/false);
 
+  // Honor color diagnostics.
+  res.getDiagnosticOpts().ShowColors = res.getFrontendOpts().showColors;
+
   return diags.getNumErrors() == numErrorsBefore;
 }
 
 /// Parses all Dialect related arguments and populates the variables
 /// options accordingly. Returns false if new errors are generated.
 static bool parseDialectArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
                              clang::DiagnosticsEngine &diags) {
   unsigned numErrorsBefore = diags.getNumErrors();
bool CompilerInvocation::createFromArgs(
   // -fno-ppc-native-vector-element-order
   if (args.hasArg(clang::driver::options::OPT_fno_ppc_native_vec_elem_order)) {
     res.loweringOpts.setNoPPCNativeVecElemOrder(true);
   }
 
   // Preserve all the remark options requested, i.e. -Rpass, -Rpass-missed or
   // -Rpass-analysis. This will be used later when processing and outputting the
   // remarks generated by LLVM in ExecuteCompilerInvocation.cpp.
-  for (auto *a : args.filtered(clang::driver::options::OPT_R_Group))
+  for (auto *a : args.filtered(clang::driver::options::OPT_R_Group)) {
+    if (a->getOption().matches(clang::driver::options::OPT_R_value_Group))
+      // This is -Rfoo=, where foo is the name of the diagnostic
+      // group. Add only the group name to the diagnostics.
+      res.getDiagnosticOpts().Remarks.push_back(
+          std::string(a->getOption().getName().drop_front(1).rtrim("=-")));
+    else
       res.getDiagnosticOpts().Remarks.push_back(a->getValue());
+  }
 
   success &= parseFrontendArgs(res.getFrontendOpts(), args, diags);
   parseTargetArgs(res.getTargetOpts(), args);
   parsePreprocessorArgs(res.getPreprocessorOpts(), args);
   parseCodeGenArgs(res.getCodeGenOpts(), args, diags);
   success &= parseDebugArgs(res.getCodeGenOpts(), args, diags);
   success &= parseSemaArgs(res, args, diags);
   success &= parseDialectArgs(res, args, diags);

flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp

if (!flang->getFrontendOpts().mlirArgs.empty()) {
     llvm::cl::ParseCommandLineOptions(numArgs + 1, args.get());
   }
 
   // If there were errors in processing arguments, don't do anything else.
   if (flang->getDiagnostics().hasErrorOccurred()) {
     return false;
   }
 
-  // Honor color diagnostics.
-  flang->getDiagnosticOpts().ShowColors = flang->getFrontendOpts().showColors;
-
   updateDiagEngineForOptRemarks(flang->getDiagnostics(),
                                 flang->getDiagnosticOpts());
 
   // Create and execute the frontend action.
   std::unique_ptr<FrontendAction> act(createFrontendAction(*flang));
   if (!act)
     return false;
 
   bool success = flang->executeAction(*act);
   return success;
 }
 
 } // namespace Fortran::frontend

flang/test/Driver/optimization-remark.f90

 ! 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=REMARKS
 ! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass -emit-llvm -o - 2>&1 | FileCheck %s --check-prefix=NO-REMARKS
 
 ! 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=NO-REMARKS
 ! RUN: %flang %s -O1 -Rno-pass-missed 2>&1 | FileCheck %s --allow-empty --check-prefix=NO-REMARKS
 ! RUN: %flang %s -O1 -Rno-pass-analysis 2>&1 | FileCheck %s --allow-empty --check-prefix=NO-REMARKS
 
+! Check error on invalid regex -Rpass message is emitted
+! RUN: not %flang %s -O1 -Rpass=[ 2>&1 | FileCheck %s --check-prefix=REGEX-INVALID
+
+! Check valid -Rpass regex
+! RUN: %flang %s -O1 -Rpass=loop 2>&1 | FileCheck %s
+
+! Check valid -Rpass-missed regex
+! RUN: %flang %s -O1 -Rpass-missed=loop 2>&1 | FileCheck %s --check-prefix=REMARKS-MISSED
+
+! Check valid -Rpass-analysis regex
+! RUN: %flang %s -O1 -Rpass-analysis=loop 2>&1 | FileCheck %s --check-prefix=REMARKS-ANALYSIS
+
 ! 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=REMARKS-MISSED
 
 ! Check full -Rpass-analysis message is emitted
 ! RUN: %flang %s -O1 -Rpass-analysis 2>&1 | FileCheck %s --check-prefix=REMARKS-ANALYSIS
 
+! REGEX-INVALID: error: in pattern '-Rpass=[': brackets ([ ]) not balanced
 ! CHECK: remark: Loop deleted because it is invariant
 ! REMARKS-MISSED: remark: loop not vectorized
 ! REMARKS-ANALYSIS: remark: loop not vectorized: instruction cannot be vectorized
 ! REMARKS: remark:
 ! NO-REMARKS-NOT: remark:
 
 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