[SPARC][Driver] Move feature mode selection to Arch/Sparc.cpp

[koachan]

This is so that it's performed also for flang and not just for clang.

This should fix #138494.

[llvmbot]

@llvm/pr-subscribers-flang-driver
@llvm/pr-subscribers-backend-sparc

@llvm/pr-subscribers-clang-driver

Author: Koakuma (koachan)

Changes

This is so that it's performed globally and not just for clang.

This should fix #138494.

---

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

4 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Arch/Sparc.cpp (+10-1)
• (modified) clang/lib/Driver/ToolChains/Arch/Sparc.h (+2-1)
• (modified) clang/lib/Driver/ToolChains/Clang.cpp (-10)
• (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+1-1)

diff --git a/clang/lib/Driver/ToolChains/Arch/Sparc.cpp b/clang/lib/Driver/ToolChains/Arch/Sparc.cpp
index 9595ee8383c85..504f110eac87c 100644
--- a/clang/lib/Driver/ToolChains/Arch/Sparc.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/Sparc.cpp
@@ -130,7 +130,8 @@ std::string sparc::getSparcTargetCPU(const Driver &D, const ArgList &Args,
   return "";
 }
 
-void sparc::getSparcTargetFeatures(const Driver &D, const ArgList &Args,
+void sparc::getSparcTargetFeatures(const Driver &D, const llvm::Triple &Triple,
+                                   const ArgList &Args,
                                    std::vector<StringRef> &Features) {
   sparc::FloatABI FloatABI = sparc::getSparcFloatABI(D, Args);
   if (FloatABI == sparc::FloatABI::Soft)
@@ -150,11 +151,19 @@ void sparc::getSparcTargetFeatures(const Driver &D, const ArgList &Args,
       Features.push_back("-popc");
   }
 
+  // Those OSes default to enabling VIS on 64-bit SPARC.
+  // See also the corresponding code for external assemblers in
+  // sparc::getSparcAsmModeForCPU().
+  bool IsSparcV9ATarget =
+      (Triple.getArch() == llvm::Triple::sparcv9) &&
+      (Triple.isOSLinux() || Triple.isOSFreeBSD() || Triple.isOSOpenBSD());
   if (Arg *A = Args.getLastArg(options::OPT_mvis, options::OPT_mno_vis)) {
     if (A->getOption().matches(options::OPT_mvis))
       Features.push_back("+vis");
     else
       Features.push_back("-vis");
+  } else if (IsSparcV9ATarget) {
+    Features.push_back("+vis");
   }
 
   if (Arg *A = Args.getLastArg(options::OPT_mvis2, options::OPT_mno_vis2)) {
diff --git a/clang/lib/Driver/ToolChains/Arch/Sparc.h b/clang/lib/Driver/ToolChains/Arch/Sparc.h
index 2b178d9df1ee3..fa25b4992cc8b 100644
--- a/clang/lib/Driver/ToolChains/Arch/Sparc.h
+++ b/clang/lib/Driver/ToolChains/Arch/Sparc.h
@@ -31,7 +31,8 @@ FloatABI getSparcFloatABI(const Driver &D, const llvm::opt::ArgList &Args);
 std::string getSparcTargetCPU(const Driver &D, const llvm::opt::ArgList &Args,
                               const llvm::Triple &Triple);
 
-void getSparcTargetFeatures(const Driver &D, const llvm::opt::ArgList &Args,
+void getSparcTargetFeatures(const Driver &D, const llvm::Triple &Triple,
+                            const llvm::opt::ArgList &Args,
                             std::vector<llvm::StringRef> &Features);
 const char *getSparcAsmModeForCPU(llvm::StringRef Name,
                                   const llvm::Triple &Triple);
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 456bfe885f354..bf5f427fbfbb4 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2731,16 +2731,6 @@ static void CollectArgsForIntegratedAssembler(Compilation &C,
     CmdArgs.push_back(MipsTargetFeature);
   }
 
-  // Those OSes default to enabling VIS on 64-bit SPARC.
-  // See also the corresponding code for external assemblers in
-  // sparc::getSparcAsmModeForCPU().
-  bool IsSparcV9ATarget =
-      (C.getDefaultToolChain().getArch() == llvm::Triple::sparcv9) &&
-      (Triple.isOSLinux() || Triple.isOSFreeBSD() || Triple.isOSOpenBSD());
-  if (IsSparcV9ATarget && SparcTargetFeatures.empty()) {
-    CmdArgs.push_back("-target-feature");
-    CmdArgs.push_back("+vis");
-  }
   for (const char *Feature : SparcTargetFeatures) {
     CmdArgs.push_back("-target-feature");
     CmdArgs.push_back(Feature);
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 651a39c03bb54..826e2ea7eb06d 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -856,7 +856,7 @@ void tools::getTargetFeatures(const Driver &D, const llvm::Triple &Triple,
   case llvm::Triple::sparc:
   case llvm::Triple::sparcel:
   case llvm::Triple::sparcv9:
-    sparc::getSparcTargetFeatures(D, Args, Features);
+    sparc::getSparcTargetFeatures(D, Triple, Args, Features);
     break;
   case llvm::Triple::r600:
   case llvm::Triple::amdgcn:

[MaskRay]

This is so that it's performed globally and not just for clang.

Can you elaborate what is "globally"?

[koachan]

This is so that it's performed globally and not just for clang.

Can you elaborate what is "globally"?

Ah, I meant for flang and other frontends too, not just clang, as per the bug report here.

[MaskRay]

Add a flang test?

[rorth]

I've now tested the (original version of) the patch on both sparc64-unknown-linux-gnu and sparcv9-sun-solaris2.11. The Linux/sparc64 build failure is gone, no regressions on either target.

Btw., prompted by this I've discovered that GCC's -mcpu=v9 default on Solaris/SPARC is way outdated: the Oracle Studio compilers default to -xarch=sparcvis2 since version 12.6 (from 2017 no less), which is -mcpu=ultrasparc3 in gcc/clang lingo. The 32 and 64-bit libc.so.1 match this. I'll update the gcc default after testing, and provide a similar patch for clang afterwards. Thanks for pointing this out!

[rorth]

/cherry-pick 38fc453

[rorth]

This needs to go to the release/21.x branch, too, to unbreak the Linux/sparc64 flang build.