[clang][cli] Store additional optimization remarks info

[jansvoboda11]

Summary

After a revision of D96274 changed DiagnosticOptions to not store all remark arguments as-written, it is no longer possible to reconstruct the arguments accurately from the class.

This is caused by the fact that for -Rpass=regexp and friends, DiagnosticOptions store only the group name pass and not regexp. This is the same representation used for the plain -Rpass argument.

Note that each argument must be generated exactly once in CompilerInvocation::generateCC1CommandLine, otherwise each subsequent call would produce more arguments than the previous one. Currently this works out because of the way RoundTrip splits the responsibilities for certain arguments based on what arguments were queried during parsing. However, this invariant breaks when we move to single round-trip for the whole CompilerInvocation.

This patch ensures that for one -Rpass=regexp argument, we don't generate two arguments (-Rpass from DiagnosticOptions and -Rpass=regexp from CodeGenOptions) by shifting the responsibility for handling both cases to CodeGenOptions. To distinguish between the cases correctly, additional information is stored in CodeGenOptions.

The CodeGenOptions parser of -Rpass[=regexp] arguments also looks at -Rno-pass and -R[no-]everything, which is necessary for generating the correct argument regardless of the ordering of CodeGenOptions/DiagnosticOptions parsing/generation.

[jansvoboda11]

Rename variable

[dexonsmith]

clang/include/clang/Basic/CodeGenOptions.h

310–311

Can this just be a Kind field in RemarkPattern?

[jansvoboda11]

Put RemarkKind in RemarkPattern.

[jansvoboda11]

That's a bit nicer.

Not sure if RemarkPattern is a good name now, as it may represent an optimization remark that doesn't have any pattern associated with it.
How about calling it OptimizationRemark and merging operator bool and operator -> into bool patternMatch(...) { return Pattern && Pattern.match(...); }?

[dexonsmith]

In D96847#2574408, @jansvoboda11 wrote:

That's a bit nicer.

Not sure if RemarkPattern is a good name now, as it may represent an optimization remark that doesn't have any pattern associated with it.
How about calling it OptimizationRemark and merging operator bool and operator -> into bool patternMatch(...) { return Pattern && Pattern.match(...); }?

Seems reasonable. Or matchesPattern(...)?

[jansvoboda11]

Rename RemarkPattern to OptRemark, simplify the API usage.

[jansvoboda11]

Comment wording.

[jansvoboda11]

In D96847#2576369, @dexonsmith wrote:

In D96847#2574408, @jansvoboda11 wrote:

That's a bit nicer.

Not sure if RemarkPattern is a good name now, as it may represent an optimization remark that doesn't have any pattern associated with it.
How about calling it OptimizationRemark and merging operator bool and operator -> into bool patternMatch(...) { return Pattern && Pattern.match(...); }?

Seems reasonable. Or matchesPattern(...)?

I chose patternMatches, which (IMO) suggests the pattern is contained within the object and the string to be matched is passed as an argument. To me, matchesPattern sounds like it's the other way around, which would contradict the code.

[jansvoboda11]

Ping.

[dexonsmith]

LGTM.

clang/include/clang/Basic/CodeGenOptions.h

public:
 
   /// The format used for serializing remarks (default: YAML)
   std::string OptRecordFormat;
 
   /// The name of the partition that symbols are assigned to, specified with
   /// -fsymbol-partition (see https://lld.llvm.org/Partitions.html).
   std::string SymbolPartition;
 
-  /// Regular expression and the string it was created from.
+  enum 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'.
+  };
+
+  /// Optimization remark with an optional regular expression pattern.
   struct RemarkPattern {
-    std::string Pattern;
+    RemarkKind Kind;
+    std::string Value;
     std::shared_ptr<llvm::Regex> Regex;
 
     explicit operator bool() const { return Regex != nullptr; }
 
     llvm::Regex *operator->() const { return Regex.get(); }
   };
 
   /// Regular expression to select optimizations for which we should enable
   /// optimization remarks. Transformation passes whose name matches this
   /// expression (and support this feature), will emit a diagnostic
   /// whenever they perform a transformation. This is enabled by the
   /// -Rpass=regexp flag.
   RemarkPattern OptimizationRemarkPattern;
 
   /// Regular expression to select optimizations for which we should enable
   /// missed optimization remarks. Transformation passes whose name matches this
   /// expression (and support this feature), will emit a diagnostic

clang/lib/Frontend/CompilerInvocation.cpp

Diags->Report(diag::err_analyzer_config_invalid_input) << "ctu-dir"
                                                            << "a filename";
 
   if (!AnOpts.ModelPath.empty() &&
       !llvm::sys::fs::is_directory(AnOpts.ModelPath))
     Diags->Report(diag::err_analyzer_config_invalid_input) << "model-path"
                                                            << "a filename";
 }
 
-/// Create a new Regex instance out of the string value in \p RpassArg.
-/// It returns the string and a pointer to the newly generated Regex instance.
-static CodeGenOptions::RemarkPattern
-GenerateOptimizationRemarkRegex(DiagnosticsEngine &Diags, ArgList &Args,
-                                Arg *RpassArg) {
-  StringRef Val = RpassArg->getValue();
+/// Generate a remark argument. This is an inverse of `ParseOptimizationRemark`.
+static void
+GenerateOptimizationRemark(SmallVectorImpl<const char *> &Args,
+                           CompilerInvocation::StringAllocator SA,
+                           OptSpecifier OptEQ, StringRef Name,
+                           const CodeGenOptions::RemarkPattern &Pattern) {
+  if (Pattern) {
+    GenerateArg(Args, OptEQ, Pattern.Value, SA);
+  } else if (Pattern.Kind == CodeGenOptions::RK_Enabled) {
+    GenerateArg(Args, OPT_R_Joined, Name, SA);
+  } else if (Pattern.Kind == CodeGenOptions::RK_Disabled) {
+    GenerateArg(Args, OPT_R_Joined, StringRef("no-") + Name, SA);
+  }
+};
+
+/// Parse a remark command line argument. It may be missing, disabled via
+/// '-Rno-group', enabled via '-Rgroup' or specified with a regular expression
+/// via '-Rgroup=regexp'. On top of that, it can be disabled/enabled globally by
+/// '-R[no-]everything'. The \p Kind and \p Pattern are initialized accordingly.
+static void ParseOptimizationRemark(DiagnosticsEngine &Diags, ArgList &Args,
+                                    OptSpecifier OptEQ, StringRef Name,
+                                    CodeGenOptions::RemarkPattern &Pattern) {
+  CodeGenOptions::RemarkPattern Result{CodeGenOptions::RK_Missing, "", nullptr};
+
+  auto InitializePattern =
+      [&Diags, &Args](const Arg *A, CodeGenOptions::RemarkPattern &Result) {
+        Result.Value = A->getValue();
+
         std::string RegexError;
-  std::shared_ptr<llvm::Regex> Pattern = std::make_shared<llvm::Regex>(Val);
-  if (!Pattern->isValid(RegexError)) {
+        Result.Regex = std::make_shared<llvm::Regex>(Result.Value);
+        if (!Result.Regex->isValid(RegexError)) {
           Diags.Report(diag::err_drv_optimization_remark_pattern)
-        << RegexError << RpassArg->getAsString(Args);
-    Pattern.reset();
+              << RegexError << A->getAsString(Args);
+          return false;
+        }
+
+        return true;
+      };
+
+  for (Arg *A : Args) {
+    if (A->getOption().matches(OPT_R_Joined)) {
+      StringRef Value = A->getValue();
+
+      if (Value == Name)
+        Result.Kind = CodeGenOptions::RK_Enabled;
+      else if (Value == "everything")
+        Result.Kind = CodeGenOptions::RK_EnabledEverything;
+      else if (Value.split('-') == std::make_pair(StringRef("no"), Name))
+        Result.Kind = CodeGenOptions::RK_Disabled;
+      else if (Value == "no-everything")
+        Result.Kind = CodeGenOptions::RK_DisabledEverything;
+    } else if (A->getOption().matches(OptEQ)) {
+      Result.Kind = CodeGenOptions::RK_WithPattern;
+      if (!InitializePattern(A, Result))
+        return;
     }
-  return {std::string(Val), Pattern};
+  }
+
+  if (Result.Kind == CodeGenOptions::RK_Disabled ||
+      Result.Kind == CodeGenOptions::RK_DisabledEverything) {
+    Result.Value = "";
+    Result.Regex = nullptr;
+  }
+  Pattern = Result;
 }
 
 static bool parseDiagnosticLevelMask(StringRef FlagName,
                                      const std::vector<std::string> &Levels,
                                      DiagnosticsEngine &Diags,
                                      DiagnosticLevelMask &M) {
   bool Success = true;
   for (const auto &Level : Levels) {
if (Opts.EnableAIXExtendedAltivecABI)
     GenerateArg(Args, OPT_mabi_EQ_vec_extabi, SA);
 
   if (!Opts.OptRecordPasses.empty())
     GenerateArg(Args, OPT_opt_record_passes, Opts.OptRecordPasses, SA);
 
   if (!Opts.OptRecordFormat.empty())
     GenerateArg(Args, OPT_opt_record_format, Opts.OptRecordFormat, SA);
 
-  if (Opts.OptimizationRemarkPattern)
-    GenerateArg(Args, OPT_Rpass_EQ, Opts.OptimizationRemarkPattern.Pattern, SA);
+  GenerateOptimizationRemark(Args, SA, OPT_Rpass_EQ, "pass",
+                             Opts.OptimizationRemarkPattern);
 
-  if (Opts.OptimizationRemarkMissedPattern)
-    GenerateArg(Args, OPT_Rpass_missed_EQ,
-                Opts.OptimizationRemarkMissedPattern.Pattern, SA);
-
-  if (Opts.OptimizationRemarkAnalysisPattern)
-    GenerateArg(Args, OPT_Rpass_analysis_EQ,
-                Opts.OptimizationRemarkAnalysisPattern.Pattern, SA);
+  GenerateOptimizationRemark(Args, SA, OPT_Rpass_missed_EQ, "pass-missed",
+                             Opts.OptimizationRemarkMissedPattern);
+
+  GenerateOptimizationRemark(Args, SA, OPT_Rpass_analysis_EQ, "pass-analysis",
+                             Opts.OptimizationRemarkAnalysisPattern);
 
   GenerateArg(Args, OPT_fdiagnostics_hotness_threshold_EQ,
               Opts.DiagnosticsHotnessThreshold
                   ? Twine(*Opts.DiagnosticsHotnessThreshold)
                   : "auto",
               SA);
 
   for (StringRef Sanitizer : serializeSanitizerKinds(Opts.SanitizeRecover))
if (Arg *A = Args.getLastArg(OPT_opt_record_passes)) {
     NeedLocTracking = true;
   }
 
   if (Arg *A = Args.getLastArg(OPT_opt_record_format)) {
     Opts.OptRecordFormat = A->getValue();
     NeedLocTracking = true;
   }
 
-  if (Arg *A = Args.getLastArg(OPT_Rpass_EQ)) {
-    Opts.OptimizationRemarkPattern =
-        GenerateOptimizationRemarkRegex(Diags, Args, A);
-    NeedLocTracking = true;
-  }
+  ParseOptimizationRemark(Diags, Args, OPT_Rpass_EQ, "pass",
+                          Opts.OptimizationRemarkPattern);
 
-  if (Arg *A = Args.getLastArg(OPT_Rpass_missed_EQ)) {
-    Opts.OptimizationRemarkMissedPattern =
-        GenerateOptimizationRemarkRegex(Diags, Args, A);
-    NeedLocTracking = true;
-  }
+  ParseOptimizationRemark(Diags, Args, OPT_Rpass_missed_EQ, "pass-missed",
+                          Opts.OptimizationRemarkMissedPattern);
 
-  if (Arg *A = Args.getLastArg(OPT_Rpass_analysis_EQ)) {
-    Opts.OptimizationRemarkAnalysisPattern =
-        GenerateOptimizationRemarkRegex(Diags, Args, A);
-    NeedLocTracking = true;
-  }
+  ParseOptimizationRemark(Diags, Args, OPT_Rpass_analysis_EQ, "pass-analysis",
+                          Opts.OptimizationRemarkAnalysisPattern);
+
+  NeedLocTracking |= Opts.OptimizationRemarkPattern ||
+                     Opts.OptimizationRemarkMissedPattern ||
+                     Opts.OptimizationRemarkAnalysisPattern;
 
   bool UsingSampleProfile = !Opts.SampleProfileFile.empty();
   bool UsingProfile = UsingSampleProfile ||
       (Opts.getProfileUse() != CodeGenOptions::ProfileNone);
 
   if (Opts.DiagnosticsWithHotness && !UsingProfile &&
       // An IR file will contain PGO as metadata
       IK.getLanguage() != Language::LLVM_IR)
#undef DIAG_OPTION_WITH_MARSHALLING
 
   for (const auto &Warning : Opts.Warnings) {
     // This option is automatically generated from UndefPrefixes.
     if (Warning == "undef-prefix")
       continue;
     Args.push_back(SA(StringRef("-W") + Warning));
   }
 
-  for (const auto &Remark : Opts.Remarks)
+  for (const auto &Remark : Opts.Remarks) {
+    // These arguments are generated from OptimizationRemark fields of
+    // CodeGenOptions.
+    StringRef IgnoredRemarks[] = {"pass",          "no-pass",
+                                  "pass-analysis", "no-pass-analysis",
+                                  "pass-missed",   "no-pass-missed"};
+    if (llvm::is_contained(IgnoredRemarks, Remark))
+      continue;
+
     Args.push_back(SA(StringRef("-R") + Remark));
   }
+}
 
 bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args,
                                 DiagnosticsEngine *Diags,
                                 bool DefaultDiagColor) {
   Optional<DiagnosticsEngine> IgnoringDiags;
   if (!Diags) {
     IgnoringDiags.emplace(new DiagnosticIDs(), new DiagnosticOptions(),
                           new IgnoringDiagConsumer());