[PATCH] D136930: [RISCV] Support -mcpu/mtune=native

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2190
     CmdArgs.push_back("-tune-cpu");
-    CmdArgs.push_back(A->getValue());
+    if (strcmp(A->getValue(), "native") == 0)
+      CmdArgs.push_back(Args.MakeArgString(llvm::sys::getHostCPUName()));
----------------
kito-cheng wrote:
> pcwang-thead wrote:
> > craig.topper wrote:
> > > Why can't we use A->getValue() == "native" here?
> > Because newly-changed support of other targets (like PowerPC) are in this form, so I think this may benefit from simplification?
> A->getValue return `const char *` rather than StringRef? maybe we could use `StringRef(A->getValue()) == "native"` like X86 to improve that?
> 
> https://llvm.org/doxygen/classllvm_1_1opt_1_1Arg.html
> 
> grep result from clang folder:
> ```
> $ grpe "\"native\"" * -R
> lib/Driver/ToolChains/Clang.cpp:    if (strcmp(A->getValue(), "native") == 0)
> lib/Driver/ToolChains/Clang.cpp:    if (strcmp(A->getValue(), "native") == 0)
> lib/Driver/ToolChains/Clang.cpp:    if (Name == "native")
> lib/Driver/ToolChains/Clang.cpp:    if (strcmp(A->getValue(), "native") == 0)
> lib/Driver/ToolChains/Clang.cpp:    if (Name == "native") {
> lib/Driver/ToolChains/CommonArgs.cpp:    // Handle "native" by examining the host. "native" isn't meaningful when
> lib/Driver/ToolChains/CommonArgs.cpp:    if (CPU == "native")
> lib/Driver/ToolChains/Arch/SystemZ.cpp:    if (CPUName == "native") {
> lib/Driver/ToolChains/Arch/X86.cpp:    if (CPU != "native")
> lib/Driver/ToolChains/Arch/X86.cpp:    if (StringRef(A->getValue()) == "native") {
> lib/Driver/ToolChains/Arch/PPC.cpp:    if (CPUName == "native") {
> lib/Driver/ToolChains/Arch/AArch64.cpp:  if (CPU == "native")
> lib/Driver/ToolChains/Arch/AArch64.cpp:  if (CPU == "native")
> lib/Driver/ToolChains/Arch/AArch64.cpp:  if (Split.first == "native")
> lib/Driver/ToolChains/Arch/AArch64.cpp:  if (MtuneLowerCase == "native")
> lib/Driver/ToolChains/Arch/Sparc.cpp:    if (CPUName == "native") {
> lib/Driver/ToolChains/Arch/ARM.cpp:  if (CPUName == "native") {
> lib/Driver/ToolChains/Arch/ARM.cpp:  if (MArch == "native") {
> lib/Driver/ToolChains/Arch/ARM.cpp:    if (MCPU == "native")
> lib/Driver/ToolChains/Arch/M68k.cpp:    if (CPUName == "native") {
> ```
Thanks. I guess strcmp makes sense then to avoid an implicit call to strlen in the StringRef constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136930