[PATCH] Add support for optimization reports.

Diego Novillo dnovillo at google.com
Mon Apr 14 08:04:22 PDT 2014



================
Comment at: include/clang/Basic/DiagnosticDriverKinds.td:119-121
@@ -117,1 +118,5 @@
+  "%0 in '%1'">;
+def warn_drv_optimization_remark_missing_loc : Warning<"optimization remarks "
+  "will not show source location information (use -gline-tables-only "
+  "-gcolumn-info to enable it)">, InGroup<BackendOptimizationRemark>;
 
----------------
Richard Smith wrote:
> I think this would make more sense as a note on the diagnostic we produce, if we ever actually produce one.
Oh, yeah, much better. Done. One question, however, this will cause the note to be emitted on every remark we print. Should I try to reduce the noise by making sure it's only emitted once?

================
Comment at: include/clang/Basic/DiagnosticFrontendKinds.td:35-42
@@ -34,1 +34,10 @@
 
+def err_fe_backend_optimization_remark: Remark<"%0">, CatBackend,
+    InGroup<BackendOptimizationRemark>;
+def warn_fe_backend_optimization_remark: Remark<"%0">, CatBackend,
+    InGroup<BackendOptimizationRemark>;
+def remark_fe_backend_optimization_remark: Remark<"%0">, CatBackend,
+    InGroup<BackendOptimizationRemark>, DefaultRemark;
+def note_fe_backend_optimization_remark: Remark<"%0">, CatBackend,
+    InGroup<BackendOptimizationRemark>;
+
----------------
Richard Smith wrote:
> I assume that these shouldn't all be `Remark`s?
> 
> `Note`s should not have an `InGroup`.
> 
> Please put a space to the left of the `:` (this file is inconsistent on this, but spaces on both sides is the established convention elsewhere).
> 
> Are you actually using any of these except for the `remark_...` form?
I fixed them to be the right type. They are not really used, except by the boilerplate macro ComputeDiagRemarkID() in lib/CodeGen/CodeGenAction.cpp. I replicated the behaviour of all the other handlers. I am not really sure what they are supposed to do.

================
Comment at: include/clang/Driver/Options.td:259
@@ -257,1 +258,3 @@
+  HelpText<"Report transformations performed by optimization passes whose "
+           "name matches the given POSIX regular expression.">;
 def S : Flag<["-"], "S">, Flags<[DriverOption,CC1Option]>, Group<Action_Group>,
----------------
Richard Smith wrote:
> No full stop here, please. (I think we're inconsistent on this too.)
Done.

================
Comment at: lib/Frontend/CompilerInvocation.cpp:526
@@ +525,3 @@
+    std::string RegexError;
+    Opts.OptimizationRemarkPattern = new llvm::Regex(Val);
+    if (!Opts.OptimizationRemarkPattern->isValid(RegexError)) {
----------------
Richard Smith wrote:
> It looks like this is leaked. Maybe use an `llvm::Optional<llvm::Regex>` instead?
I tried but llvm::Regex has an explicitly deleted constructor. I'm not sure how to deal with that, so I ended up using regular pointer semantics.


http://reviews.llvm.org/D3226






More information about the cfe-commits mailing list