-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
[flang][driver] Limit the usage of -mvscale-max and -mvscale-min #77905
[flang][driver] Limit the usage of -mvscale-max and -mvscale-min #77905
Conversation
Make sure that `-mvscale-max` and `-mvscale-min` are only available for targets that are known to support vscale and scalable vectors. Also fix capitalization of function variables.
@llvm/pr-subscribers-flang-driver Author: Andrzej Warzyński (banach-space) ChangesMake sure that Also fix capitalization of function variables. Full diff: https://github.com/llvm/llvm-project/pull/77905.diff 2 Files Affected:
diff --git a/flang/lib/Frontend/CompilerInvocation.cpp b/flang/lib/Frontend/CompilerInvocation.cpp
index 0732f4bef290f4..a3c41fb4611f56 100644
--- a/flang/lib/Frontend/CompilerInvocation.cpp
+++ b/flang/lib/Frontend/CompilerInvocation.cpp
@@ -1056,28 +1056,45 @@ static bool parseFloatingPointArgs(CompilerInvocation &invoc,
/// \param [out] diags DiagnosticsEngine to report erros with
static bool parseVScaleArgs(CompilerInvocation &invoc, llvm::opt::ArgList &args,
clang::DiagnosticsEngine &diags) {
+ const auto *vscaleMin =
+ args.getLastArg(clang::driver::options::OPT_mvscale_min_EQ);
+ const auto *vscaleMax =
+ args.getLastArg(clang::driver::options::OPT_mvscale_max_EQ);
+
+ if (!vscaleMin && !vscaleMax)
+ return true;
+
+ llvm::Triple triple = llvm::Triple(invoc.getTargetOpts().triple);
+ if (!triple.isAArch64() && !triple.isRISCV()) {
+ const unsigned diagID =
+ diags.getCustomDiagID(clang::DiagnosticsEngine::Error,
+ "`-mvscale-max` and `-mvscale-min` are not "
+ "supported for this architecture: %0");
+ diags.Report(diagID) << triple.getArchName();
+ return false;
+ }
+
LangOptions &opts = invoc.getLangOpts();
- if (const auto arg =
- args.getLastArg(clang::driver::options::OPT_mvscale_min_EQ)) {
- llvm::StringRef argValue = llvm::StringRef(arg->getValue());
- unsigned VScaleMin;
- if (argValue.getAsInteger(/*Radix=*/10, VScaleMin)) {
+ if (vscaleMin) {
+ llvm::StringRef argValue = llvm::StringRef(vscaleMin->getValue());
+ unsigned vscaleMinVal;
+ if (argValue.getAsInteger(/*Radix=*/10, vscaleMinVal)) {
diags.Report(clang::diag::err_drv_unsupported_option_argument)
- << arg->getSpelling() << argValue;
+ << vscaleMax->getSpelling() << argValue;
return false;
}
- opts.VScaleMin = VScaleMin;
+ opts.VScaleMin = vscaleMinVal;
}
- if (const auto arg =
- args.getLastArg(clang::driver::options::OPT_mvscale_max_EQ)) {
- llvm::StringRef argValue = llvm::StringRef(arg->getValue());
- unsigned VScaleMax;
- if (argValue.getAsInteger(/*Radix=w*/ 10, VScaleMax)) {
+
+ if (vscaleMax) {
+ llvm::StringRef argValue = llvm::StringRef(vscaleMax->getValue());
+ unsigned vscaleMaxVal;
+ if (argValue.getAsInteger(/*Radix=w*/ 10, vscaleMaxVal)) {
diags.Report(clang::diag::err_drv_unsupported_option_argument)
- << arg->getSpelling() << argValue;
+ << vscaleMax->getSpelling() << argValue;
return false;
}
- opts.VScaleMax = VScaleMax;
+ opts.VScaleMax = vscaleMaxVal;
}
return true;
}
diff --git a/flang/test/Driver/unsupported-vscale-max-min.f90 b/flang/test/Driver/unsupported-vscale-max-min.f90
new file mode 100644
index 00000000000000..dffeb54b665b50
--- /dev/null
+++ b/flang/test/Driver/unsupported-vscale-max-min.f90
@@ -0,0 +1,8 @@
+! REQUIRES: x86-registered-target
+
+! RUN: not %flang_fc1 -triple x86_64-unknown-linux-gnu -mvscale-min=1 -mvscale-max=1 -fsyntax-only %s 2>&1 | FileCheck %s
+
+! CHECK: `-mvscale-max` and `-mvscale-min` are not supported for this architecture: x86_64
+
+subroutine func
+end subroutine func
|
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
(I wish there was an easier way to simply remove the options when not supported, but I don't think there is a way to do that)
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
After llvm#77905, setting -mvscale-min or -mvscale-max on targets other than AArch64 and RISC-V should be an error now, so we no longer need this target-agnostic code in getVScaleRange.
…m#77905) Make sure that `-mvscale-max` and `-mvscale-min` are only available for targets that are known to support vscale and scalable vectors. Also fix capitalization of function variables.
llvm#78133) After llvm#77905, setting -mvscale-min or -mvscale-max on targets other than AArch64 and RISC-V should be an error now, so we no longer need this target-agnostic code in getVScaleRange.
Make sure that
-mvscale-max
and-mvscale-min
are only available fortargets that are known to support vscale and scalable vectors.
Also fix capitalization of function variables.