[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH
uweigand added a comment. Thanks for working on this, @thakis ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75914/new/ https://reviews.llvm.org/D75914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH
xnox added a comment. Sorry for being too slow =) I like all the fixes done to move the define into a more natural config.h location, thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75914/new/ https://reviews.llvm.org/D75914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH
thakis added inline comments. Comment at: clang/CMakeLists.txt:311 + "SystemZ Default Arch") +add_definitions( -DCLANG_SYSTEMZ_DEFAULT_ARCH="${CLANG_SYSTEMZ_DEFAULT_ARCH}") + thakis wrote: > Hahnfeld wrote: > > 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? > +1 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. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75914/new/ https://reviews.llvm.org/D75914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH
thakis added inline comments. Comment at: clang/CMakeLists.txt:311 + "SystemZ Default Arch") +add_definitions( -DCLANG_SYSTEMZ_DEFAULT_ARCH="${CLANG_SYSTEMZ_DEFAULT_ARCH}") + thakis wrote: > thakis wrote: > > Hahnfeld wrote: > > > 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? > > +1 > 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. 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.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75914/new/ https://reviews.llvm.org/D75914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH
thakis added inline comments. Comment at: clang/CMakeLists.txt:311 + "SystemZ Default Arch") +add_definitions( -DCLANG_SYSTEMZ_DEFAULT_ARCH="${CLANG_SYSTEMZ_DEFAULT_ARCH}") + Hahnfeld wrote: > 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? +1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75914/new/ https://reviews.llvm.org/D75914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH
uweigand added a comment. Ah, good point. Dimitry, can you prepare an updated patch to implement Jonas' suggestion? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75914/new/ https://reviews.llvm.org/D75914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH
Hahnfeld added inline comments. Comment at: clang/CMakeLists.txt:311 + "SystemZ Default Arch") +add_definitions( -DCLANG_SYSTEMZ_DEFAULT_ARCH="${CLANG_SYSTEMZ_DEFAULT_ARCH}") + 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? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75914/new/ https://reviews.llvm.org/D75914 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75914: systemz: allow configuring default CLANG_SYSTEMZ_ARCH
This revision was automatically updated to reflect the committed changes. Closed by commit rG9c9d88d8b1bb: [SystemZ] Allow configuring default CLANG_SYSTEMZ_ARCH (authored by uweigand). Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75914/new/ https://reviews.llvm.org/D75914 Files: clang/CMakeLists.txt clang/lib/Driver/ToolChains/Arch/SystemZ.cpp Index: clang/lib/Driver/ToolChains/Arch/SystemZ.cpp === --- clang/lib/Driver/ToolChains/Arch/SystemZ.cpp +++ clang/lib/Driver/ToolChains/Arch/SystemZ.cpp @@ -47,7 +47,7 @@ return std::string(CPUName); } - return "z10"; + return CLANG_SYSTEMZ_DEFAULT_ARCH; } void systemz::getSystemZTargetFeatures(const Driver , const ArgList , Index: clang/CMakeLists.txt === --- clang/CMakeLists.txt +++ clang/CMakeLists.txt @@ -306,6 +306,10 @@ "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.") Index: clang/lib/Driver/ToolChains/Arch/SystemZ.cpp === --- clang/lib/Driver/ToolChains/Arch/SystemZ.cpp +++ clang/lib/Driver/ToolChains/Arch/SystemZ.cpp @@ -47,7 +47,7 @@ return std::string(CPUName); } - return "z10"; + return CLANG_SYSTEMZ_DEFAULT_ARCH; } void systemz::getSystemZTargetFeatures(const Driver , const ArgList , Index: clang/CMakeLists.txt === --- clang/CMakeLists.txt +++ clang/CMakeLists.txt @@ -306,6 +306,10 @@ "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.") ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits