[PATCH] D68049: Propeller: Clang options for basic block sections

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

Clang side LG with some minor changes.



================
Comment at: clang/docs/ClangCommandLineReference.rst:1336
+
+Generate labels for each basic block or place each basic block or a subset of basic blocks in its own section
+
----------------
This file is automatically generated from the .td file; this text will be lost when the file is auto-generated. To include a custom description here, it should be specified as a `DocBrief` annotation on the option. (Otherwise we default to using the `HelpText`.)


================
Comment at: clang/docs/UsersManual.rst:1704
+  Controls whether Clang emits a label for each basic block.  Further, with
+  values "all" and "list=arg", each basic block or a subset of basic blocks
+  can be placed in its own unique section.
----------------
Please either explain here or link to an explanation elsewhere of what "arg" is and how to format it.


================
Comment at: clang/lib/CodeGen/BackendUtil.cpp:503-504
+    if (!MBOrErr)
+      errs() << "Error loading basic block sections function list file: "
+             << MBOrErr.getError().message() << "\n";
+    else
----------------
rsmith wrote:
> Please emit a proper diagnostic for this rather than writing to stderr directly. We should be able to pass the `DiagnosticsEngine` into here.
Can you register a regular diagnostic message for this (it looks like we register CodeGen diagnostics in include/clang/Basic/DiagnosticFrontendKinds.td) rather than emitting a custom diagnostic?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68049/new/

https://reviews.llvm.org/D68049