[PATCH] D146054: [RISCV] Add --print-supported-extensions support

[Brandon Wu via Phabricator via cfe-commits]

4vtomat added inline comments.


================
Comment at: clang/include/clang/Frontend/FrontendOptions.h:290
+  /// Output time trace profile.
+  unsigned TimeTrace : 1;
+
----------------
MaskRay wrote:
> stray change?
Oh, maybe it was added accidentally during rebase lol~


================
Comment at: clang/test/Driver/print-supported-extensions.c:10
+// CHECK:All available -march extensions for RISC-V
+// CHECK:	Name                Version
+// CHECK-NEXT:	i                   2.1
----------------
MaskRay wrote:
> For `--strict-whitespace --match-full-lines` testing, we usually right align `CHECK` and `CHECK-NEXT`.
Oh, I got it, thanks!


================
Comment at: clang/tools/driver/cc1_main.cpp:187
+/// Print supported extensions of the RISCV target.
+static void printSupportedExtensions() {
+  llvm::riscvExtensionsHelp();
----------------
MaskRay wrote:
> The call site should just call riscvExtensionsHelp so that this internal linkage function can be avoided.
Yeah, good idea!


================
Comment at: clang/tools/driver/cc1_main.cpp:187
+/// Print supported extensions of the RISCV target.
+static int print_supported_extensions() {
+  llvm::riscvExtensionsHelp();
----------------
MaskRay wrote:
> `printSupportedExtensions`. Why does this function return a dummy return value?
> 
> You can do `return printSupportedExtensions(), 0` below to save some lines.
This is a good idea, return in its caller is much clear for it's intension!


================
Comment at: llvm/lib/Support/RISCVISAInfo.cpp:202
+void llvm::riscvExtensionsHelp() {
+  errs() << "All available -march extensions for RISC-V\n\n";
+  errs() << '\t' << left_justify("Name", 20) << "Version\n";
----------------
MaskRay wrote:
> I think `outs()` is more conventional. Most `gcc --print-*` options go to stdout. `clang --print-supported-cpus` deviates and we should not copy its issue.
Sure, I agree to keep it conventional, since I was following `--print-supported-cpus` so I made it this way, thank you for pointing out this!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146054