-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang][Driver][RISCV] Rename getRISCFeaturesFromMcpu
. NFCI
#162545
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[clang][Driver][RISCV] Rename getRISCFeaturesFromMcpu
. NFCI
#162545
Conversation
@llvm/pr-subscribers-backend-risc-v @llvm/pr-subscribers-clang Author: Min-Yih Hsu (mshockwave) ChangesThis 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 NFCI. Full diff: https://github.com/llvm/llvm-project/pull/162545.diff 1 Files Affected:
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;
|
@llvm/pr-subscribers-clang-driver Author: Min-Yih Hsu (mshockwave) ChangesThis 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 NFCI. Full diff: https://github.com/llvm/llvm-project/pull/162545.diff 1 Files Affected:
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;
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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 byriscv::getRISCVTargetFeatures
-- it is only checking if the-mcpu
value is valid or not now. Therefore, this patch renames it intoisValidRISCVCPU
and exits early if it's not.NFCI.