Skip to content

Conversation

mshockwave
Copy link
Member

This function, which has a typo in the name btw, is no longer doing what it was created to do: instead of deducting non-extension target features from -mcpu -- a task that was (more or less) subsumed by riscv::getRISCVTargetFeatures -- it is only checking if the -mcpu value is valid or not now. Therefore, this patch renames it into isValidRISCVCPU and exits early if it's not.

NFCI.

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' labels Oct 8, 2025
@llvmbot
Copy link
Member

llvmbot commented Oct 8, 2025

@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-clang

Author: Min-Yih Hsu (mshockwave)

Changes

This function, which has a typo in the name btw, is no longer doing what it was created to do: instead of deducting non-extension target features from -mcpu -- a task that was (more or less) subsumed by riscv::getRISCVTargetFeatures -- it is only checking if the -mcpu value is valid or not now. Therefore, this patch renames it into isValidRISCVCPU and exits early if it's not.

NFCI.


Full diff: https://github.com/llvm/llvm-project/pull/162545.diff

1 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Arch/RISCV.cpp (+6-6)
diff --git a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index 76dde0da8e849..f2e79e71f93d4 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -49,11 +49,8 @@ static bool getArchFeatures(const Driver &D, StringRef Arch,
   return true;
 }
 
-// Get features except standard extension feature
-static void getRISCFeaturesFromMcpu(const Driver &D, const Arg *A,
-                                    const llvm::Triple &Triple,
-                                    StringRef Mcpu,
-                                    std::vector<StringRef> &Features) {
+static bool isValidRISCVCPU(const Driver &D, const Arg *A,
+                            const llvm::Triple &Triple, StringRef Mcpu) {
   bool Is64Bit = Triple.isRISCV64();
   if (!llvm::RISCV::parseCPU(Mcpu, Is64Bit)) {
     // Try inverting Is64Bit in case the CPU is valid, but for the wrong target.
@@ -63,7 +60,9 @@ static void getRISCFeaturesFromMcpu(const Driver &D, const Arg *A,
     else
       D.Diag(clang::diag::err_drv_unsupported_option_argument)
           << A->getSpelling() << Mcpu;
+    return false;
   }
+  return true;
 }
 
 void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
@@ -84,7 +83,8 @@ void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
     if (CPU == "native")
       CPU = llvm::sys::getHostCPUName();
 
-    getRISCFeaturesFromMcpu(D, A, Triple, CPU, Features);
+    if (!isValidRISCVCPU(D, A, Triple, CPU))
+      return;
 
     if (llvm::RISCV::hasFastScalarUnalignedAccess(CPU))
       CPUFastScalarUnaligned = true;

@llvmbot
Copy link
Member

llvmbot commented Oct 8, 2025

@llvm/pr-subscribers-clang-driver

Author: Min-Yih Hsu (mshockwave)

Changes

This function, which has a typo in the name btw, is no longer doing what it was created to do: instead of deducting non-extension target features from -mcpu -- a task that was (more or less) subsumed by riscv::getRISCVTargetFeatures -- it is only checking if the -mcpu value is valid or not now. Therefore, this patch renames it into isValidRISCVCPU and exits early if it's not.

NFCI.


Full diff: https://github.com/llvm/llvm-project/pull/162545.diff

1 Files Affected:

  • (modified) clang/lib/Driver/ToolChains/Arch/RISCV.cpp (+6-6)
diff --git a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
index 76dde0da8e849..f2e79e71f93d4 100644
--- a/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -49,11 +49,8 @@ static bool getArchFeatures(const Driver &D, StringRef Arch,
   return true;
 }
 
-// Get features except standard extension feature
-static void getRISCFeaturesFromMcpu(const Driver &D, const Arg *A,
-                                    const llvm::Triple &Triple,
-                                    StringRef Mcpu,
-                                    std::vector<StringRef> &Features) {
+static bool isValidRISCVCPU(const Driver &D, const Arg *A,
+                            const llvm::Triple &Triple, StringRef Mcpu) {
   bool Is64Bit = Triple.isRISCV64();
   if (!llvm::RISCV::parseCPU(Mcpu, Is64Bit)) {
     // Try inverting Is64Bit in case the CPU is valid, but for the wrong target.
@@ -63,7 +60,9 @@ static void getRISCFeaturesFromMcpu(const Driver &D, const Arg *A,
     else
       D.Diag(clang::diag::err_drv_unsupported_option_argument)
           << A->getSpelling() << Mcpu;
+    return false;
   }
+  return true;
 }
 
 void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
@@ -84,7 +83,8 @@ void riscv::getRISCVTargetFeatures(const Driver &D, const llvm::Triple &Triple,
     if (CPU == "native")
       CPU = llvm::sys::getHostCPUName();
 
-    getRISCFeaturesFromMcpu(D, A, Triple, CPU, Features);
+    if (!isValidRISCVCPU(D, A, Triple, CPU))
+      return;
 
     if (llvm::RISCV::hasFastScalarUnalignedAccess(CPU))
       CPUFastScalarUnaligned = true;

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mshockwave mshockwave merged commit 86ad98c into llvm:main Oct 8, 2025
12 of 15 checks passed
@mshockwave mshockwave deleted the patch/riscv/rename-getriscvfeaturesfrommcpu-nfc branch October 8, 2025 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:RISC-V clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants