================
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to