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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits