Skip to content
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] Remove dead -mvscale-{min,max} logic from getVScaleRange. NFCI #78133

Merged
merged 1 commit into from
Jan 15, 2024

Conversation

lukel97
Copy link
Contributor

@lukel97 lukel97 commented Jan 15, 2024

After #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.

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.
@llvmbot llvmbot added flang:driver flang Flang issues not falling into any other category labels Jan 15, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 15, 2024

@llvm/pr-subscribers-flang-driver

Author: Luke Lau (lukel97)

Changes

After #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.


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

1 Files Affected:

  • (modified) flang/lib/Frontend/FrontendActions.cpp (+2-5)
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 8fb1998f01703a..74e3992d5ab62b 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -769,7 +769,6 @@ getRISCVVScaleRange(CompilerInstance &ci) {
 // too much of clang, so for now, replicate the functionality.
 static std::optional<std::pair<unsigned, unsigned>>
 getVScaleRange(CompilerInstance &ci) {
-  const auto &langOpts = ci.getInvocation().getLangOpts();
   const llvm::Triple triple(ci.getInvocation().getTargetOpts().triple);
 
   if (triple.isAArch64())
@@ -777,10 +776,8 @@ getVScaleRange(CompilerInstance &ci) {
   if (triple.isRISCV())
     return getRISCVVScaleRange(ci);
 
-  if (langOpts.VScaleMin || langOpts.VScaleMax)
-    return std::pair<unsigned, unsigned>(
-        langOpts.VScaleMin ? langOpts.VScaleMin : 1, langOpts.VScaleMax);
-
+  // All other architectures that don't support scalable vectors (i.e. don't
+  // need vscale)
   return std::nullopt;
 }
 

Copy link
Contributor

@banach-space banach-space left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@lukel97 lukel97 merged commit 219c14a into llvm:main Jan 15, 2024
6 checks passed
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:driver flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants