[PATCH] Add support for optimization reports.

Richard Smith richard at metafoo.co.uk
Mon Mar 31 15:06:13 PDT 2014



================
Comment at: include/clang/Basic/DiagnosticGroups.td:665
@@ -664,1 +664,2 @@
 def RemarkBackendPlugin : DiagGroup<"remark-backend-plugin">;
+def BackendOptimizationReport : DiagGroup<"Rpass">;
----------------
Do you intend for this to be controlled by `-WRpass`? I don't think you should add a `DiagGroup` for this one at all.

================
Comment at: include/clang/Driver/Options.td:257-258
@@ -255,3 +256,4 @@
 def Q : Flag<["-"], "Q">;
-def R : Flag<["-"], "R">;
+def Rpass_EQ : Joined<["-"], "Rpass=">, Group<R_Group>, Flags<[CC1Option]>,
+  HelpText<"Report transformations performed by optimization passes">;
 def S : Flag<["-"], "S">, Flags<[DriverOption,CC1Option]>, Group<Action_Group>,
----------------
Should we have a `-Rno-pass=...` too?

================
Comment at: lib/CodeGen/CodeGenAction.cpp:387-389
@@ +386,5 @@
+    SourceLocation Loc;
+    // FIXME: This should be done earlier.
+    Diags.setDiagnosticMapping(diag::remark_fe_backend_optimization_report,
+                               diag::Mapping::MAP_REMARK, Loc);
+    // FIXME: There should be a way of transferring the DILocation
----------------
Maybe add a `DefaultRemark` to `Diagnostic.td`, and then add `DefaultRemark` to the `def` for your `Remark` to enable it by default, and drop this?

================
Comment at: lib/CodeGen/CodeGenAction.cpp:393-396
@@ +392,6 @@
+    // unnecessarily.
+    DILocation DIL(D.getDebugLoc().getAsMDNode(D.getFunction().getContext()));
+    StringRef FileName = DIL.getFilename();
+    unsigned LineNum = DIL.getLineNumber();
+    unsigned ColumnNum = DIL.getColumnNumber();
+    Diags.Report(Loc, diag::remark_fe_backend_optimization_report)
----------------
Yuck. :( It's not correct to rely on diagnostics being formatted this way: we support several output formats that don't use the `'file:line:col: '` formatting.

Could we annotate the raw form of Clang's `SourceLocation` onto the IR so that we can use it here? Failing that, you can map the filename/line/column back to a `SourceLocation` by asking the `clang::SourceManager` nicely (see `SourceManager::translateFileLineCol` and `FileManager::getFile`).

================
Comment at: lib/CodeGen/CodeGenAction.cpp:397-399
@@ +396,5 @@
+    unsigned ColumnNum = DIL.getColumnNumber();
+    Diags.Report(Loc, diag::remark_fe_backend_optimization_report)
+        << Twine(FileName + ":" + Twine(LineNum) + ":" + Twine(ColumnNum) +
+                 ": " + D.getMsg()).str();
+  }
----------------
I think this'll report the wrong warning flag, something like:

  remark: something happened [-WRpass]

Ideally we should report

  remark: something happened [-Rpass=passname]

You'll probably need to add an optional warning flag name override to `DiagnosticsEngine` to support this (and use it as an override in `printDiagnosticOptions` in `TextDiagnosticPrinter.cpp`).


http://llvm-reviews.chandlerc.com/D3226



More information about the cfe-commits mailing list