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

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. Reviewed By: awarzynski Differential Revision: https://reviews.llvm.org/D158436
llvm · Aug 31, 2023 · 8e315c6 · 8e315c6
1 parent a84b09f
commit 8e315c6
Show file tree

Hide file tree

Showing 5 changed files with 119 additions and 41 deletions.
diff --git a/flang/include/flang/Frontend/CodeGenOptions.h b/flang/include/flang/Frontend/CodeGenOptions.h
@@ -73,11 +73,12 @@ class CodeGenOptions : public CodeGenOptionsBase {
   // 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,
-                 // -Rpass-analysis
-    RK_Disabled, // Remark disabled via '-Rno-group', i.e. -Rno-pass,
-                 // -Rno-pass-missed, -Rno-pass-analysis.
+    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,
+                    // -Rno-pass-missed, -Rno-pass-analysis.
+    RK_WithPattern, // Remark pattern specified via '-Rgroup=regexp'.
   };
 
   /// Optimization remark with an optional regular expression pattern.

diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -156,11 +156,12 @@ static bool parseDebugArgs(Fortran::frontend::CodeGenOptions &opts,
 // 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.");
+         "Unsupported remark option name provided.");
   CodeGenOptions::OptRemark result;
 
   for (llvm::opt::Arg *a : args) {
@@ -180,6 +181,17 @@ parseOptimizationRemark(llvm::opt::ArgList &args,
         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;
@@ -240,15 +252,20 @@ static void parseCodeGenArgs(Fortran::frontend::CodeGenOptions &opts,
 
   // 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,
+                              /*remarkOptName=*/"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,
+      /*remarkOptName=*/"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,
+      /*remarkOptName=*/"pass-analysis");
 
   if (auto *a = args.getLastArg(clang::driver::options::OPT_save_temps_EQ))
     opts.SaveTempsDir = a->getValue();
@@ -746,6 +763,9 @@ static bool parseDiagArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
   res.getFrontendOpts().showColors =
       parseShowColorsArgs(args, /*defaultDiagColor=*/false);
 
+  // Honor color diagnostics.
+  res.getDiagnosticOpts().ShowColors = res.getFrontendOpts().showColors;
+
   return diags.getNumErrors() == numErrorsBefore;
 }
 
@@ -1010,8 +1030,18 @@ bool CompilerInvocation::createFromArgs(
   // 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))
-    res.getDiagnosticOpts().Remarks.push_back(a->getValue());
+  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 remark option name to the diagnostics. e.g. for
+      // -Rpass= we will add the string "pass".
+      res.getDiagnosticOpts().Remarks.push_back(
+          std::string(a->getOption().getName().drop_front(1).rtrim("=-")));
+    else
+      // If no regex was provided, add the provided value, e.g. for -Rpass add
+      // the string "pass".
+      res.getDiagnosticOpts().Remarks.push_back(a->getValue());
+  }
 
   success &= parseFrontendArgs(res.getFrontendOpts(), args, diags);
   parseTargetArgs(res.getTargetOpts(), args);

diff --git a/flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp b/flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp
@@ -191,9 +191,6 @@ bool executeCompilerInvocation(CompilerInstance *flang) {
     return false;
   }
 
-  // Honor color diagnostics.
-  flang->getDiagnosticOpts().ShowColors = flang->getFrontendOpts().showColors;
-
   updateDiagEngineForOptRemarks(flang->getDiagnostics(),
                                 flang->getDiagnosticOpts());
 

diff --git a/flang/test/Driver/optimization-remark-invalid.f90 b/flang/test/Driver/optimization-remark-invalid.f90
@@ -0,0 +1,12 @@
+! This file tests invalid usage of 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 error on invalid regex -Rpass message is emitted
+! RUN: not %flang %s -O1 -Rpass=[ -c 2>&1 | FileCheck %s --check-prefix=REGEX-INVALID
+
+
+! REGEX-INVALID: error: in pattern '-Rpass=[': brackets ([ ]) not balanced
+
+program forttest
+end program forttest
diff --git a/flang/test/Driver/optimization-remark.f90 b/flang/test/Driver/optimization-remark.f90
@@ -4,46 +4,84 @@
 
 ! DEFINE: %{output} = -emit-llvm -o /dev/null 2>&1
 
+! Check fc1 can handle -Rpass
+! RUN: %flang_fc1 %s -O1 -Rpass %{output} 2>&1 | FileCheck %s --check-prefix=REMARKS
+
 ! Check that we can override -Rpass= with -Rno-pass.
-! RUN: %flang_fc1 %s -O1 -Rpass %{output} | FileCheck %s --check-prefix=REMARKS
-! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass  %{output} | FileCheck %s --allow-empty --check-prefix=NO-REMARKS
+! RUN: %flang_fc1 %s -O1 -Rpass -Rno-pass %{output} 2>&1 | FileCheck %s --allow-empty --check-prefix=NO-REMARKS
 
 ! Check -Rno-pass, -Rno-pass-analysis, -Rno-pass-missed nothing emitted
-! RUN: %flang %s -O1 -Rno-pass -S %{output} | FileCheck %s --allow-empty --check-prefix=NO-REMARKS
-! RUN: %flang %s -O1 -Rno-pass-missed -S %{output}  | FileCheck %s --allow-empty --check-prefix=NO-REMARKS
-! RUN: %flang %s -O1 -Rno-pass-analysis -S %{output} | FileCheck %s --allow-empty --check-prefix=NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass -S %{output} 2>&1 | FileCheck %s --allow-empty --check-prefix=NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-missed -S %{output} 2>&1 | FileCheck %s --allow-empty --check-prefix=NO-REMARKS
+! RUN: %flang %s -O1 -Rno-pass-analysis -S %{output} 2>&1 | FileCheck %s --allow-empty --check-prefix=NO-REMARKS
+
+! Check valid -Rpass regex
+! RUN: %flang %s -O1 -Rpass=loop -S %{output} 2>&1 | FileCheck %s --check-prefix=PASS-REGEX-LOOP-ONLY
+
+! Check valid -Rpass-missed regex
+! RUN: %flang %s -O1 -Rpass-missed=loop -S %{output} 2>&1 | FileCheck %s --check-prefix=MISSED-REGEX-LOOP-ONLY
+
+! Check valid -Rpass-analysis regex
+! RUN: %flang %s -O1 -Rpass-analysis=loop -S %{output} 2>&1 | FileCheck %s --check-prefix=ANALYSIS-REGEX-LOOP-ONLY
 
 ! Check full -Rpass message is emitted
-! RUN: %flang %s -O1 -Rpass  -S %{output} | FileCheck %s
+! RUN: %flang %s -O1 -Rpass -S %{output} 2>&1 | FileCheck %s --check-prefix=PASS
 
 ! Check full -Rpass-missed message is emitted
-! RUN: %flang %s -O1 -Rpass-missed  -S %{output} | FileCheck %s --check-prefix=REMARKS-MISSED
+! RUN: %flang %s -O1 -Rpass-missed -S %{output} 2>&1 | FileCheck %s --check-prefix=MISSED
 
 ! Check full -Rpass-analysis message is emitted
-! RUN: %flang %s -O1 -Rpass-analysis  -S %{output} | FileCheck %s --check-prefix=REMARKS-ANALYSIS
-
-! CHECK: remark: Loop deleted because it is invariant
-! REMARKS-MISSED: {{.*}} will not be inlined into {{.*}} because its definition is unavailable
-! REMARKS-MISSED: remark: loop not vectorized
-! REMARKS-MISSED-NOT: loop not vectorized: instruction cannot be vectorized
-! REMARKS-ANALYSIS: remark: loop not vectorized: instruction cannot be vectorized
-! REMARKS-ANALYSIS-NOT: {{.*}} will not be inlined into {{.*}} because its definition is unavailable
+! RUN: %flang %s -O1 -Rpass-analysis -S -o /dev/null 2>&1 | FileCheck %s --check-prefix=ANALYSIS
 
 ! 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
+! With plain -Rpass, -Rpass-missed or -Rpass-analysis, we expect remarks related to 2 opportunities (loop vectorisation / loop delete and load hoisting).
+! Once we start filtering, this is reduced to 1 one of the loop passes.
+
+! PASS-REGEX-LOOP-ONLY-NOT:     remark: hoisting load
+! PASS-REGEX-LOOP-ONLY:         remark: Loop deleted because it is invariant
+
+! MISSED-REGEX-LOOP-ONLY-NOT:   remark: failed to hoist load with loop-invariant address because load is conditionally executed
+! MISSED-REGEX-LOOP-ONLY:       remark: loop not vectorized
+
+
+! ANALYSIS-REGEX-LOOP-ONLY:     remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
+! ANALYSIS-REGEX-LOOP-ONLY:     Unknown data dependence.
+! ANALYSIS-REGEX-LOOP-ONLY-NOT: remark:{{.*}}: IR instruction count changed from {{[0-9]+}} to {{[0-9]+}}; Delta: {{-?[0-9]+}}
+
+! PASS:                         remark: hoisting load
+! PASS:                         remark: Loop deleted because it is invariant
+
+! MISSED:                       remark: failed to hoist load with loop-invariant address because load is conditionally executed
+! MISSED:                       remark: loop not vectorized
+! MISSED-NOT:                   remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
+! MISSED-NOT:                   Unknown data dependence.
+
+! ANALYSIS:                     remark: loop not vectorized: unsafe dependent memory operations in loop. Use #pragma loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
+! ANALYSIS:                     Unknown data dependence.
+! ANALYSIS:                     remark: {{.*}}: IR instruction count changed from {{[0-9]+}} to {{[0-9]+}}; Delta: {{-?[0-9]+}}
+! ANALYSIS-NOT:                 remark: failed to hoist load with loop-invariant address because load is conditionally executed
+
+subroutine swap_real(a1, a2)
+   implicit none
+
+   real, dimension(1:2) :: aR1
+   integer :: i, n
+   real, intent(inout) :: a1(:), a2(:)
+   real :: a
+
+!  Swap
+   do i = 1, min(size(a1), size(a2))
+      a = a1(i)
+      a1(i) = a2(i)
+      a2(i) = a
+   end do
 
-    do n = 1,50
+! Do a random loop to generate a successful loop-delete pass
+    do n = 1,2
         aR1(n) = n * 1.34
     end do
 
-end program forttest
+end subroutine swap_real