[z/OS] Set the default arch for z/OS to be arch10

[perry-ca]

The default arch level on z/OS is arch10. Update the code so z/OS has arch10 without changing the default for zLinux.

[llvmbot]

@llvm/pr-subscribers-backend-systemz
@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Sean Perry (perry-ca)

Changes

The default arch level on z/OS is arch10. Update the code so z/OS has arch10 without changing the default for zLinux.

---

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

8 Files Affected:

• (modified) clang/CMakeLists.txt (+2)
• (modified) clang/include/clang/Config/config.h.cmake (+3)
• (modified) clang/lib/Basic/Targets/SystemZ.h (+3-4)
• (modified) clang/lib/Driver/ToolChains/Arch/SystemZ.cpp (+4-1)
• (modified) clang/lib/Driver/ToolChains/Arch/SystemZ.h (+2-1)
• (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+1-1)
• (modified) clang/lib/Driver/ToolChains/Gnu.cpp (+2-1)
• (modified) clang/test/Preprocessor/predefined-arch-macros.c (+16)

diff --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index f092766fa19f07..f06fbec1d02f91 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -270,6 +270,8 @@ set(CLANG_DEFAULT_OPENMP_RUNTIME "libomp" CACHE STRING
 
 set(CLANG_SYSTEMZ_DEFAULT_ARCH "z10" CACHE STRING "SystemZ Default Arch")
 
+set(CLANG_SYSTEMZ_ZOS_DEFAULT_ARCH "zEC12" CACHE STRING "SystemZ z/OS Default Arch")
+
 set(CLANG_VENDOR ${PACKAGE_VENDOR} CACHE STRING
   "Vendor-specific text for showing with version information.")
 
diff --git a/clang/include/clang/Config/config.h.cmake b/clang/include/clang/Config/config.h.cmake
index 27ed69e21562bf..004c5c95c00cff 100644
--- a/clang/include/clang/Config/config.h.cmake
+++ b/clang/include/clang/Config/config.h.cmake
@@ -32,6 +32,9 @@
 /* Default architecture for SystemZ. */
 #define CLANG_SYSTEMZ_DEFAULT_ARCH "${CLANG_SYSTEMZ_DEFAULT_ARCH}"
 
+/* Default architecture for SystemZ for z/OS. */
+#define CLANG_SYSTEMZ_ZOS_DEFAULT_ARCH "${CLANG_SYSTEMZ_ZOS_DEFAULT_ARCH}"
+
 /* Multilib basename for libdir. */
 #define CLANG_INSTALL_LIBDIR_BASENAME "${CLANG_INSTALL_LIBDIR_BASENAME}"
 
diff --git a/clang/lib/Basic/Targets/SystemZ.h b/clang/lib/Basic/Targets/SystemZ.h
index 8e302acd51b8ad..45fb7e447b6429 100644
--- a/clang/lib/Basic/Targets/SystemZ.h
+++ b/clang/lib/Basic/Targets/SystemZ.h
@@ -24,7 +24,6 @@ namespace targets {
 class LLVM_LIBRARY_VISIBILITY SystemZTargetInfo : public TargetInfo {
 
   static const char *const GCCRegNames[];
-  std::string CPU;
   int ISARevision;
   bool HasTransactionalExecution;
   bool HasVector;
@@ -33,7 +32,8 @@ class LLVM_LIBRARY_VISIBILITY SystemZTargetInfo : public TargetInfo {
 
 public:
   SystemZTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
-      : TargetInfo(Triple), CPU("z10"), ISARevision(8),
+      : TargetInfo(Triple),
+        ISARevision(getISARevision(Triple.isOSzOS() ? "zEC12" : "z10")),
         HasTransactionalExecution(false), HasVector(false), SoftFloat(false),
         UnalignedSymbols(false) {
     IntMaxType = SignedLong;
@@ -140,8 +140,7 @@ class LLVM_LIBRARY_VISIBILITY SystemZTargetInfo : public TargetInfo {
   }
 
   bool setCPU(const std::string &Name) override {
-    CPU = Name;
-    ISARevision = getISARevision(CPU);
+    ISARevision = getISARevision(Name);
     return ISARevision != -1;
   }
 
diff --git a/clang/lib/Driver/ToolChains/Arch/SystemZ.cpp b/clang/lib/Driver/ToolChains/Arch/SystemZ.cpp
index 2213f431eb8114..7baeffc00053a7 100644
--- a/clang/lib/Driver/ToolChains/Arch/SystemZ.cpp
+++ b/clang/lib/Driver/ToolChains/Arch/SystemZ.cpp
@@ -34,7 +34,8 @@ systemz::FloatABI systemz::getSystemZFloatABI(const Driver &D,
   return ABI;
 }
 
-std::string systemz::getSystemZTargetCPU(const ArgList &Args) {
+std::string systemz::getSystemZTargetCPU(const Driver &, const ArgList &Args,
+                                         const llvm::Triple &T) {
   if (const Arg *A = Args.getLastArg(clang::driver::options::OPT_march_EQ)) {
     llvm::StringRef CPUName = A->getValue();
 
@@ -48,6 +49,8 @@ std::string systemz::getSystemZTargetCPU(const ArgList &Args) {
 
     return std::string(CPUName);
   }
+  if (T.isOSzOS())
+    return CLANG_SYSTEMZ_ZOS_DEFAULT_ARCH;
   return CLANG_SYSTEMZ_DEFAULT_ARCH;
 }
 
diff --git a/clang/lib/Driver/ToolChains/Arch/SystemZ.h b/clang/lib/Driver/ToolChains/Arch/SystemZ.h
index 1e42b68a8f3c20..438dab8765bc34 100644
--- a/clang/lib/Driver/ToolChains/Arch/SystemZ.h
+++ b/clang/lib/Driver/ToolChains/Arch/SystemZ.h
@@ -27,7 +27,8 @@ enum class FloatABI {
 
 FloatABI getSystemZFloatABI(const Driver &D, const llvm::opt::ArgList &Args);
 
-std::string getSystemZTargetCPU(const llvm::opt::ArgList &Args);
+std::string getSystemZTargetCPU(const Driver &D, const llvm::opt::ArgList &Args,
+                                const llvm::Triple &T);
 
 void getSystemZTargetFeatures(const Driver &D, const llvm::opt::ArgList &Args,
                               std::vector<llvm::StringRef> &Features);
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index b65b96db16bd79..73f5abfaafcba0 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -604,7 +604,7 @@ std::string tools::getCPUName(const Driver &D, const ArgList &Args,
     return getLanaiTargetCPU(Args);
 
   case llvm::Triple::systemz:
-    return systemz::getSystemZTargetCPU(Args);
+    return systemz::getSystemZTargetCPU(D, Args, T);
 
   case llvm::Triple::r600:
   case llvm::Triple::amdgcn:
diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp
index f55b8bf48c13f7..149f4060f4fc76 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -936,7 +936,8 @@ void tools::gnutools::Assembler::ConstructJob(Compilation &C,
   case llvm::Triple::systemz: {
     // Always pass an -march option, since our default of z10 is later
     // than the GNU assembler's default.
-    std::string CPUName = systemz::getSystemZTargetCPU(Args);
+    std::string CPUName =
+        systemz::getSystemZTargetCPU(D, Args, getToolChain().getTriple());
     CmdArgs.push_back(Args.MakeArgString("-march=" + CPUName));
     break;
   }
diff --git a/clang/test/Preprocessor/predefined-arch-macros.c b/clang/test/Preprocessor/predefined-arch-macros.c
index ca51f2fc22c517..55a6cf63a94e11 100644
--- a/clang/test/Preprocessor/predefined-arch-macros.c
+++ b/clang/test/Preprocessor/predefined-arch-macros.c
@@ -4153,6 +4153,20 @@
 
 // Begin SystemZ/GCC/Linux tests ----------------
 
+// RUN: %clang -E -dM %s -o - 2>&1 \
+// RUN:     -target s390x-ibm-zos \
+// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_SYSTEMZ_ZOS
+// CHECK_SYSTEMZ_ZOS: #define __ARCH__ 10
+// CHECK_SYSTEMZ_ZOS: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_1 1
+// CHECK_SYSTEMZ_ZOS: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
+// CHECK_SYSTEMZ_ZOS: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
+// CHECK_SYSTEMZ_ZOS: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
+// CHECK_SYSTEMZ_ZOS: #define __HTM__ 1
+// CHECK_SYSTEMZ_ZOS: #define __LONG_DOUBLE_128__ 1
+// CHECK_SYSTEMZ_ZOS: #define __s390__ 1
+// CHECK_SYSTEMZ_ZOS: #define __s390x__ 1
+// CHECK_SYSTEMZ_ZOS: #define __zarch__ 1
+
 // RUN: %clang -march=arch8 -E -dM %s -o - 2>&1 \
 // RUN:     -target s390x-unknown-linux \
 // RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_SYSTEMZ_ARCH8
@@ -4164,6 +4178,7 @@
 // CHECK_SYSTEMZ_ARCH8: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
 // CHECK_SYSTEMZ_ARCH8: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
 // CHECK_SYSTEMZ_ARCH8: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
+// CHECK_SYSTEMZ_ARCH8-NOT: #define __HTM__ 1
 // CHECK_SYSTEMZ_ARCH8: #define __LONG_DOUBLE_128__ 1
 // CHECK_SYSTEMZ_ARCH8: #define __s390__ 1
 // CHECK_SYSTEMZ_ARCH8: #define __s390x__ 1
@@ -4180,6 +4195,7 @@
 // CHECK_SYSTEMZ_ARCH9: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_2 1
 // CHECK_SYSTEMZ_ARCH9: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_4 1
 // CHECK_SYSTEMZ_ARCH9: #define __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8 1
+// CHECK_SYSTEMZ_ARCH9-NOT: #define __HTM__ 1
 // CHECK_SYSTEMZ_ARCH9: #define __LONG_DOUBLE_128__ 1
 // CHECK_SYSTEMZ_ARCH9: #define __s390__ 1
 // CHECK_SYSTEMZ_ARCH9: #define __s390x__ 1

[MaskRay]

We are trying to remove such cmake variables in favor of configuration file.

[perry-ca]

We are trying to remove such cmake variables in favor of configuration file.

I'm not sure which configuration file(s) you are referring to. I assume you mean the cache files in clang/cmake/caches. If so, I don't think we want to put it there because CLANG_SYSTEMZ_ZOS_DEFAULT_ARCH is a value defined for all the builds. It's not a value controlling how the build is done and we aren't trying to override the value of CLANG_SYSTEMZ_DEFAULT_ARCH for a particular build configuration.

I might need to explain some more. If the user compiles with -target=s390x-unknown-linux then the default arch is arch8/z10. If the user compiles with -target=s390x-ibm-zos then the default arch is arch10/zEC12. The arch value is determined at runtime when clang is run. I could see us using a cache file to override the value of CLANG_SYSTEMZ_DEFAULT_ARCH if both values weren't needed by the compiler at the same time.

[perry-ca]

ping @MaskRay. Thanks

[MaskRay]

Clang configuration files https://clang.llvm.org/docs/UsersManual.html#configuration-files cover your use case and the feature is exactly designed to avoid such cmake default configs. https://blogs.gentoo.org/mgorny/2022/10/07/clang-in-gentoo-now-sets-default-runtimes-via-config-file/

You can add -march=... in clang.cfg beside the clang executable.

[perry-ca]

@MaskRay Got it.

The problem with that solution is that if you use --target you won't get the correct arch. This would be a problem for any cross compilation. For example, say you cross compile from zLinux (which wouldn't have the config file), the arch would be arch8. And if you cross compiled from z/OS (with the config file) to zLinux then the default arch would be arch10. Both of these would be incorrect.

We also use the config files for installation specific information. It is common for users to have their own config files. If we require that the option be in the config file then we would be creating a very error prone situation.

We need to be able to change the arch default based on the target triple.

[perry-ca]

ping @MaskRay ^^^

[perry-ca]

I removed the ability to make z/OS default arch configurable at build time. We don't really need it and this makes the code simpler.

[MaskRay]

@MaskRay Got it.

The problem with that solution is that if you use --target you won't get the correct arch. This would be a problem for any cross compilation. For example, say you cross compile from zLinux (which wouldn't have the config file), the arch would be arch8. And if you cross compiled from z/OS (with the config file) to zLinux then the default arch would be arch10. Both of these would be incorrect.

We also use the config files for installation specific information. It is common for users to have their own config files. If we require that the option be in the config file then we would be creating a very error prone situation.

We need to be able to change the arch default based on the target triple.

Sorry for the late reply. Such driver defaults via cmake variable would make testing brittle. Users expect that check-clang pass regardless of the cmake variable setting. If a test changes behavior due to different -march=, there are a few choices:

• add a REQUIRES: zos-new-default-march directive
• hard code a -march=

Neither is immediately noticeable. In the past clang/test/Driver has had many such tests that require fixup. We have tried removing some unnecessary CLANG_DEFAULT_*.

[perry-ca]

@MaskRay no worries. This brittle test issue was a reason I trying to keep the default for zLinux unchanged by this.

I have posted a new commit that eliminates the cmake variable for z/OS.

[MaskRay]

unneeded include

[perry-ca]

This is needed for the change in SystemZ.h to replace the hard coded "z10" string with the CLANG_SYSTEMZ_DEFAULT_ARCH cmake variable. This applies to the other comment too.

[MaskRay]

If SystemZ.h uses CLANG_SYSTEMZ_DEFAULT_ARCH, that file should have #include "clang/Config/config.h" as IWYU

[perry-ca]

That was my preference too except clang/Config/config.h has a clause that it can only be included once and will generate an error if it is included a second time.

[MaskRay]

unneeded include

[MaskRay]

@MaskRay Got it.
The problem with that solution is that if you use --target you won't get the correct arch. This would be a problem for any cross compilation. For example, say you cross compile from zLinux (which wouldn't have the config file), the arch would be arch8. And if you cross compiled from z/OS (with the config file) to zLinux then the default arch would be arch10. Both of these would be incorrect.
We also use the config files for installation specific information. It is common for users to have their own config files. If we require that the option be in the config file then we would be creating a very error prone situation.
We need to be able to change the arch default based on the target triple.

Sorry for the late reply. Such driver defaults via cmake variable would make testing brittle. Users expect that check-clang pass regardless of the cmake variable setting. If a test changes behavior due to different -march=, there are a few choices:

• add a REQUIRES: zos-new-default-march directive
• hard code a -march=

Neither is immediately noticeable. In the past clang/test/Driver has had many such tests that require fixup. We have tried removing some unnecessary CLANG_DEFAULT_*.

I just realized that https://reviews.llvm.org/D75914 added CLANG_SYSTEMZ_DEFAULT_ARCH for Ubuntu. @xnox @uweigand

I think we want to avoid such CMake options/clang config.h

As an alternative, if the clang executable is at /tmp/Rel/bin/clang, you can add /tmp/Rel/bin/s390x-unknown-linux-gnu.cfg with content -march=z13.

Once we drop CLANG_SYSTEMZ_DEFAULT_ARCH, the config.h include will not be needed.

[xnox]

@FHe

@MaskRay Got it.
The problem with that solution is that if you use --target you won't get the correct arch. This would be a problem for any cross compilation. For example, say you cross compile from zLinux (which wouldn't have the config file), the arch would be arch8. And if you cross compiled from z/OS (with the config file) to zLinux then the default arch would be arch10. Both of these would be incorrect.
We also use the config files for installation specific information. It is common for users to have their own config files. If we require that the option be in the config file then we would be creating a very error prone situation.
We need to be able to change the arch default based on the target triple.

Sorry for the late reply. Such driver defaults via cmake variable would make testing brittle. Users expect that check-clang pass regardless of the cmake variable setting. If a test changes behavior due to different -march=, there are a few choices:

• add a REQUIRES: zos-new-default-march directive
• hard code a -march=

Neither is immediately noticeable. In the past clang/test/Driver has had many such tests that require fixup. We have tried removing some unnecessary CLANG_DEFAULT_*.

I just realized that https://reviews.llvm.org/D75914 added CLANG_SYSTEMZ_DEFAULT_ARCH for Ubuntu. @xnox @uweigand

I think we want to avoid such CMake options/clang config.h

As an alternative, if the clang executable is at /tmp/Rel/bin/clang, you can add /tmp/Rel/bin/s390x-unknown-linux-gnu.cfg with content -march=z13.

Once we drop CLANG_SYSTEMZ_DEFAULT_ARCH, the config.h include will not be needed.

I don't understand your proposal. In Ubuntu we need to set built-in default arch value, to a higher one than upstream has it at, for all builds without any toolchain configs. Such that any builds with any toolchain configs by default target a higher arch setting.

arch10 is way too old and slow.

and z13 might not be enough either for latest devel distributions, i.e. next development ubuntu release may be bumping further above z13.

Does your proposal address above needs?

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[perry-ca]

I removed the additional use of CLANG_SYSTEMZ_DEFAULT_ARCH and left that for a separate discussion. The only code clean up left here is the elimination of the CPU member that wasn't used.

[MaskRay]

@FHe

@MaskRay Got it.
The problem with that solution is that if you use --target you won't get the correct arch. This would be a problem for any cross compilation. For example, say you cross compile from zLinux (which wouldn't have the config file), the arch would be arch8. And if you cross compiled from z/OS (with the config file) to zLinux then the default arch would be arch10. Both of these would be incorrect.
We also use the config files for installation specific information. It is common for users to have their own config files. If we require that the option be in the config file then we would be creating a very error prone situation.
We need to be able to change the arch default based on the target triple.

Sorry for the late reply. Such driver defaults via cmake variable would make testing brittle. Users expect that check-clang pass regardless of the cmake variable setting. If a test changes behavior due to different -march=, there are a few choices:

• add a REQUIRES: zos-new-default-march directive
• hard code a -march=

Neither is immediately noticeable. In the past clang/test/Driver has had many such tests that require fixup. We have tried removing some unnecessary CLANG_DEFAULT_*.

I just realized that reviews.llvm.org/D75914 added CLANG_SYSTEMZ_DEFAULT_ARCH for Ubuntu. @xnox @uweigand
I think we want to avoid such CMake options/clang config.h
As an alternative, if the clang executable is at /tmp/Rel/bin/clang, you can add /tmp/Rel/bin/s390x-unknown-linux-gnu.cfg with content -march=z13.
Once we drop CLANG_SYSTEMZ_DEFAULT_ARCH, the config.h include will not be needed.

I don't understand your proposal. In Ubuntu we need to set built-in default arch value, to a higher one than upstream has it at, for all builds without any toolchain configs. Such that any builds with any toolchain configs by default target a higher arch setting.

arch10 is way too old and slow.

and z13 might not be enough either for latest devel distributions, i.e. next development ubuntu release may be bumping further above z13.

Does your proposal address above needs?

Yes. See this part:

As an alternative, if the clang executable is at /tmp/Rel/bin/clang, you can add /tmp/Rel/bin/s390x-unknown-linux-gnu.cfg with content -march=z13.

See also https://blogs.gentoo.org/mgorny/2022/10/07/clang-in-gentoo-now-sets-default-runtimes-via-config-file/

We should then remove CLANG_SYSTEMZ_DEFAULT_ARCH (https://reviews.llvm.org/D75914)

[MaskRay]

--target= for new driver tests.

However, this does not test clangDriver and therefore %clang_cc1 is preferred.

(You can ignore previous lines that obey the convention. I can fix them later.)

[xnox]

Requiring a config file when previously was not needed is a regression in behaviour for distributions.

I guess Ubuntu would have to continue to patch this back in.

Built-in defaults exist for a reason and yes at times need to be higher.

This change risks performance regressions for RHEL SUSE and Ubuntu. Please call out this change in release notes.