systemz: allow configuring default CLANG_SYSTEMZ_ARCH

[• xnox]

Summary

On Ubuntu, we want to raise default CLANG_SYSTEMZ_ARCH to z13, thus allow configuring this via CMake.
On Debian, we want to raise it to z196.

[uweigand]

Thanks for working on this! A few comments inline.

CMakeLists.txt

311 ↗

(On Diff #249333)

Should be moved above the set(CLANG_VENDOR, ... line.

Also, this new variable itself have a set line, so that we can provide a descriptive text, and also move the "z10" default to here instead of the sources.

Finally, I think the mixed-case name doesn't really match the common naming conventions here, what about CLANG_SYSTEMZ_DEFAULT_ARCH ?

[• xnox]

In D75914#1914571, @uweigand wrote:

Thanks for working on this! A few comments inline.

Updated! @uweigand Please re-review.

[uweigand]

LGTM, thanks!

[• xnox]

I cannot commit the patch myself, please "sponsor" committing this change.

[uweigand]

This doesn't apply cleanly to current mainline. Can you rebase and test again? I'll check it in then.

[• xnox]

Rebased I think. If it helps here is the github pull-request too https://github.com/llvm/llvm-project/pull/196

Git format patch https://github.com/xnox/llvm-project/commit/a6122cd8dc1b10bcba751f851cf5b1751cff165c.patch

Or $ git pull https://github.com/xnox/llvm-project D75914

[Hahnfeld]

clang/CMakeLists.txt

311

Passing values like this is unusual for CMake and causes all source files to be recompiled if the value is changed. Instead could we add this to include/clang/Config/config.h.cmake like other CLANG_DEFAULT_* options?

[uweigand]

Ah, good point. Dimitry, can you prepare an updated patch to implement Jonas' suggestion?

[thakis]

clang/CMakeLists.txt

311

+1

[thakis]

clang/CMakeLists.txt

311

Also, this is used in one cpp file; passing the define to every compilation unit seems a bit aggressive.

I'll try to land a patch to fix this.

[thakis]

clang/CMakeLists.txt

311

Done in c506adcdf2ca3ba6459e52e09c55868e3b57af46. (This only changes the implementation of the feature; the way people who build clang tell cmake what they want didn't change.)

[• xnox]

Sorry for being too slow =) I like all the fixes done to move the define into a more natural config.h location, thank you.

[uweigand]

Thanks for working on this, @thakis !

clang/CMakeLists.txt

set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_35" CACHE STRING
   "Default architecture for OpenMP offloading to Nvidia GPUs.")
 string(REGEX MATCH "^sm_([0-9]+)$" MATCHED_ARCH "${CLANG_OPENMP_NVPTX_DEFAULT_ARCH}")
 if (NOT DEFINED MATCHED_ARCH OR "${CMAKE_MATCH_1}" LESS 35)
   message(WARNING "Resetting default architecture for OpenMP offloading to Nvidia GPUs to sm_35")
   set(CLANG_OPENMP_NVPTX_DEFAULT_ARCH "sm_35" CACHE STRING
     "Default architecture for OpenMP offloading to Nvidia GPUs." FORCE)
 endif()
 
+set(CLANG_SYSTEMZ_DEFAULT_ARCH "z10" CACHE STRING
+  "SystemZ Default Arch")
+add_definitions( -DCLANG_SYSTEMZ_DEFAULT_ARCH="${CLANG_SYSTEMZ_DEFAULT_ARCH}")
+
 set(CLANG_VENDOR ${PACKAGE_VENDOR} CACHE STRING
   "Vendor-specific text for showing with version information.")
 
 if( CLANG_VENDOR )
   add_definitions( -DCLANG_VENDOR="${CLANG_VENDOR} " )
 endif()
 
 set(CLANG_REPOSITORY_STRING "" CACHE STRING

clang/lib/Driver/ToolChains/Arch/SystemZ.cpp

if (CPUName == "native") {
       if (!CPU.empty() && CPU != "generic")
         return CPU;
       else
         return "";
     }
 
     return std::string(CPUName);
   }
-  return "z10";
+  return CLANG_SYSTEMZ_DEFAULT_ARCH;
 }
 
 void systemz::getSystemZTargetFeatures(const Driver &D, const ArgList &Args,
                                        std::vector<llvm::StringRef> &Features) {
   // -m(no-)htm overrides use of the transactional-execution facility.
   if (Arg *A = Args.getLastArg(options::OPT_mhtm, options::OPT_mno_htm)) {
     if (A->getOption().matches(options::OPT_mhtm))
       Features.push_back("+transactional-execution");