From bcd2ca1ef872f4bbd3faf83d4b73dffeacc5ca34 Mon Sep 17 00:00:00 2001
From: Ben Cumming <bcumming@cscs.ch>
Date: Wed, 23 Jun 2021 12:02:41 +0200
Subject: [PATCH] Add optional target-specific configuration for CMake (#1586)

- Add an optional CMake option `ARB_CXX_FLAGS_TARGET` for setting target-specific flags to use when compiling for the target architecture (not applied to compilation of modcc).
- If `ARB_ARCH=="none"` CMake will not add architecture-specific `mtune/march/mcpu` flags
- Remove `ARB_CXXOPT_ARCH` from the installed `arbor-config.cmake`, and replace with more general `ARB_CXX_FLAGS_TARGET`.
- Update spack `package.py` to use this feature to pass custom flags.

Fixes #1519
Fixes #1522

Replaces PR #1518
---
 CMakeLists.txt                         | 23 +++++++++++++++++++----
 cmake/arbor-config.cmake.in            |  2 +-
 doc/internals/extending_catalogues.rst |  1 +
 mechanisms/BuildModules.cmake          |  7 +++----
 mechanisms/CMakeLists.txt              |  3 +++
 python/test/cpp/CMakeLists.txt         |  2 +-
 scripts/build-catalogue                |  1 +
 spack/package.py                       |  6 ++++--
 sup/CMakeLists.txt                     |  2 +-
 test/ubench/CMakeLists.txt             |  2 +-
 test/unit-distributed/CMakeLists.txt   |  4 ++--
 test/unit/CMakeLists.txt               |  3 ++-
 12 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index afd3510d..7ecb0a47 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -42,6 +42,13 @@ set_property(CACHE PROPERTY STRINGS "none" "cuda" "cuda-clang" "hip")
 
 option(ARB_USE_BUNDLED_LIBS "Use bundled 3rd party libraries" OFF)
 
+# Optional additional CXX Flags used for all code that will run on the target
+# CPU architecture. Recorded in installed target, for downstream dependencies
+# to use.
+# Useful, for example, when a user wants to compile with target-specific
+# optimization flags.
+set(ARB_CXX_FLAGS_TARGET "" CACHE STRING "Optional additional flags for compilation")
+
 #----------------------------------------------------------
 # Debug support
 #----------------------------------------------------------
@@ -230,11 +237,19 @@ set(arbor_supported_components)
 
 # Target microarchitecture for building arbor libraries, tests and examples
 #---------------------------------------------------------------------------
-if(ARB_ARCH)
-    set_arch_target(ARB_CXXOPT_ARCH "${ARB_ARCH}")
-    target_compile_options(arbor-private-deps INTERFACE ${ARB_CXXOPT_ARCH})
-    target_compile_options(arborenv-private-deps INTERFACE ${ARB_CXXOPT_ARCH})
+
+# Set the full set of target flags in ARB_CXX_FLAGS_TARGET_FULL, which
+# will include target-specific -march flags if ARB_ARCH is not "none".
+if(ARB_ARCH STREQUAL "none")
+    set(ARB_CXX_FLAGS_TARGET_FULL ${ARB_CXX_FLAGS_TARGET})
+else()
+    set_arch_target(ARB_CXXOPT_ARCH ${ARB_ARCH})
+    set(ARB_CXX_FLAGS_TARGET_FULL ${ARB_CXX_FLAGS_TARGET} ${ARB_CXXOPT_ARCH})
 endif()
+separate_arguments(ARB_CXX_FLAGS_TARGET_FULL)
+
+target_compile_options(arbor-private-deps INTERFACE ${ARB_CXX_FLAGS_TARGET_FULL})
+target_compile_options(arborenv-private-deps INTERFACE ${ARB_CXX_FLAGS_TARGET_FULL})
 
 # Profiling and test features
 #-----------------------------
diff --git a/cmake/arbor-config.cmake.in b/cmake/arbor-config.cmake.in
index d7b056d9..5ddbc0cc 100644
--- a/cmake/arbor-config.cmake.in
+++ b/cmake/arbor-config.cmake.in
@@ -49,7 +49,7 @@ set(ARB_ARCH @ARB_ARCH@)
 set(ARB_MODCC_FLAGS @ARB_MODCC_FLAGS@)
 set(ARB_CXX @CMAKE_CXX_COMPILER@)
 set(ARB_CXX_FLAGS @CMAKE_CXX_FLAGS@)
-set(ARB_CXXOPT_ARCH @ARB_CXXOPT_ARCH@)
+set(ARB_CXX_FLAGS_TARGET @ARB_CXX_FLAGS_TARGET_FULL@)
 
 _append_property(arbor::arbor INTERFACE_LINK_LIBRARIES @arbor_add_import_libs@)
 _append_property(arbor::arborenv INTERFACE_LINK_LIBRARIES @arborenv_add_import_libs@)
diff --git a/doc/internals/extending_catalogues.rst b/doc/internals/extending_catalogues.rst
index 5ac03748..b9de2e88 100644
--- a/doc/internals/extending_catalogues.rst
+++ b/doc/internals/extending_catalogues.rst
@@ -27,6 +27,7 @@ catalogues (*default*, *bbp*, and *allen*). The required steps are as follows
        NAME <catalogue-name>                             # Name of your catalogue
        SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/<directory>" # Directory name (added above)
        OUTPUT "<output-name>"                            # Variable name to output to
+       CXX_FLAGS_TARGET "<compiler flags>"               # Target-specific flags for C++ compiler
        MECHS <names>)                                    # Space separated list of mechanism
                                                          # names w/o .mod suffix.
 
diff --git a/mechanisms/BuildModules.cmake b/mechanisms/BuildModules.cmake
index b1770356..aa751769 100644
--- a/mechanisms/BuildModules.cmake
+++ b/mechanisms/BuildModules.cmake
@@ -58,7 +58,7 @@ function(build_modules)
 endfunction()
 
 function("make_catalogue")
-  cmake_parse_arguments(MK_CAT "" "NAME;SOURCES;OUTPUT;ARBOR;STANDALONE;VERBOSE" "MECHS" ${ARGN})
+  cmake_parse_arguments(MK_CAT "" "NAME;SOURCES;OUTPUT;ARBOR;STANDALONE;VERBOSE" "CXX_FLAGS_TARGET;MECHS" ${ARGN})
   set(MK_CAT_OUT_DIR "${CMAKE_CURRENT_BINARY_DIR}/generated/${MK_CAT_NAME}")
 
   # Need to set ARB_WITH_EXTERNAL_MODCC *and* modcc
@@ -74,7 +74,7 @@ function("make_catalogue")
     message("Catalogue output:     ${MK_CAT_OUT_DIR}")
     message("Arbor source tree:    ${MK_CAT_ARBOR}")
     message("Build as standalone:  ${MK_CAT_STANDALONE}")
-    message("Arbor arch:           ${ARB_CXXOPT_ARCH}")
+    message("Arbor cxx flags:      ${MK_CAT_CXX_FLAGS_TARGET}")
     message("Arbor cxx compiler:   ${ARB_CXX}")
     message("Current cxx compiler: ${CMAKE_CXX_COMPILER}")
   endif()
@@ -118,11 +118,10 @@ function("make_catalogue")
   endforeach()
   set(${MK_CAT_OUTPUT} ${catalogue_${MK_CAT_NAME}_source} PARENT_SCOPE)
 
-  set_source_files_properties(${catalogue_${MK_CAT_NAME}_source} COMPILE_FLAGS ${ARB_CXXOPT_ARCH})
-
   if(${MK_CAT_STANDALONE})
     add_library(${MK_CAT_NAME}-catalogue SHARED ${catalogue_${MK_CAT_NAME}_source})
     target_compile_definitions(${MK_CAT_NAME}-catalogue PUBLIC STANDALONE=1)
+    target_compile_options(${MK_CAT_NAME}-catalogue PUBLIC ${MK_CAT_CXX_FLAGS_TARGET})
     set_target_properties(${MK_CAT_NAME}-catalogue
       PROPERTIES
       SUFFIX ".so"
diff --git a/mechanisms/CMakeLists.txt b/mechanisms/CMakeLists.txt
index 5d57d327..67e5f9cc 100644
--- a/mechanisms/CMakeLists.txt
+++ b/mechanisms/CMakeLists.txt
@@ -6,6 +6,7 @@ make_catalogue(
   SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/bbp"
   OUTPUT "CAT_BBP_SOURCES"
   MECHS CaDynamics_E2 Ca_HVA Ca_LVAst Ih Im K_Pst K_Tst Nap_Et2 NaTa_t NaTs2_t SK_E2 SKv3_1
+  CXX_FLAGS_TARGET "${ARB_CXX_FLAGS_TARGET_FULL}"
   ARBOR "${PROJECT_SOURCE_DIR}"
   STANDALONE FALSE
   VERBOSE ${ARB_CAT_VERBOSE})
@@ -15,6 +16,7 @@ make_catalogue(
   SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/allen"
   OUTPUT "CAT_ALLEN_SOURCES"
   MECHS CaDynamics Ca_HVA Ca_LVA Ih Im Im_v2 K_P K_T Kd Kv2like Kv3_1 NaTa NaTs NaV Nap SK
+  CXX_FLAGS_TARGET "${ARB_CXX_FLAGS_TARGET_FULL}"
   ARBOR "${PROJECT_SOURCE_DIR}"
   STANDALONE FALSE
   VERBOSE ${ARB_CAT_VERBOSE})
@@ -24,6 +26,7 @@ make_catalogue(
   SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/default"
   OUTPUT "CAT_DEFAULT_SOURCES"
   MECHS exp2syn expsyn expsyn_stdp hh kamt kdrmt nax nernst pas
+  CXX_FLAGS_TARGET "${ARB_CXX_FLAGS_TARGET_FULL}"
   ARBOR "${PROJECT_SOURCE_DIR}"
   STANDALONE FALSE
   VERBOSE ${ARB_CAT_VERBOSE})
diff --git a/python/test/cpp/CMakeLists.txt b/python/test/cpp/CMakeLists.txt
index 98c72c75..fc460292 100644
--- a/python/test/cpp/CMakeLists.txt
+++ b/python/test/cpp/CMakeLists.txt
@@ -12,7 +12,7 @@ add_dependencies(tests py_unit)
 add_library(py_unit_lib $<TARGET_OBJECTS:pyarb_obj>)
 target_link_libraries(py_unit_lib PRIVATE arbor pybind11::module)
 
-target_compile_options(py_unit PRIVATE ${ARB_CXXOPT_ARCH})
+target_compile_options(py_unit PRIVATE ${ARB_CXX_FLAGS_TARGET_FULL})
 target_include_directories(
     py_unit PRIVATE
     "${CMAKE_CURRENT_BINARY_DIR}"
diff --git a/scripts/build-catalogue b/scripts/build-catalogue
index d474156e..6351b800 100755
--- a/scripts/build-catalogue
+++ b/scripts/build-catalogue
@@ -92,6 +92,7 @@ make_catalogue(
   SOURCES "${{CMAKE_CURRENT_SOURCE_DIR}}/mod"
   OUTPUT "CAT_{name.upper()}_SOURCES"
   MECHS {' '.join(mods)}
+  CXX_FLAGS_TARGET ${{ARB_CXX_FLAGS_TARGET}}
   ARBOR {arb}
   STANDALONE ON
   VERBOSE {"ON" if verbose else "OFF"})
diff --git a/spack/package.py b/spack/package.py
index 43019fa6..e83c8993 100644
--- a/spack/package.py
+++ b/spack/package.py
@@ -70,7 +70,9 @@ class Arbor(CMakePackage, CudaPackage):
         if '+cuda' in self.spec:
             args.append('-DARB_GPU=cuda')
 
-        # rely on spack's compiler wrapper to set architecture
-        args.append('-DARB_ARCH=')
+        # query spack for the architecture-specific compiler flags set by its wrapper
+        args.append('-DARB_ARCH=none')
+        opt_flags = self.spec.target.optimization_flags(self.spec.compiler.name, self.spec.compiler.version)
+        args.append('-DARB_CXX_FLAGS_TARGET='+opt_flags)
 
         return args
diff --git a/sup/CMakeLists.txt b/sup/CMakeLists.txt
index 79627717..596888bb 100644
--- a/sup/CMakeLists.txt
+++ b/sup/CMakeLists.txt
@@ -7,7 +7,7 @@ set(sup-sources
 add_library(arbor-sup ${sup-sources})
 
 # Compile sup library with the same optimization flags as libarbor.
-target_compile_options(arbor-sup PRIVATE ${ARB_CXXOPT_ARCH})
+target_compile_options(arbor-sup PRIVATE ${ARB_CXX_FLAGS_TARGET_FULL})
 
 # The sup library uses both the json library and libarbor
 target_link_libraries(arbor-sup PUBLIC ${json_library_name} arbor)
diff --git a/test/ubench/CMakeLists.txt b/test/ubench/CMakeLists.txt
index 9b7d5407..adea5647 100644
--- a/test/ubench/CMakeLists.txt
+++ b/test/ubench/CMakeLists.txt
@@ -26,7 +26,7 @@ foreach(bench_src ${bench_sources})
     add_executable(${bench_exe} EXCLUDE_FROM_ALL "${bench_src}")
 
     target_link_libraries(${bench_exe} arbor arborio arbor-private-headers ext-benchmark)
-    target_compile_options(${bench_exe} PRIVATE ${ARB_CXXOPT_ARCH})
+    target_compile_options(${bench_exe} PRIVATE ${ARB_CXX_FLAGS_TARGET_FULL})
     target_compile_definitions(${bench_exe} PRIVATE "-DDATADIR=\"${CMAKE_CURRENT_SOURCE_DIR}/../swc\"")
     list(APPEND bench_exe_list ${bench_exe})
 endforeach()
diff --git a/test/unit-distributed/CMakeLists.txt b/test/unit-distributed/CMakeLists.txt
index 8defe961..0ec9fb9e 100644
--- a/test/unit-distributed/CMakeLists.txt
+++ b/test/unit-distributed/CMakeLists.txt
@@ -11,7 +11,7 @@ set(unit-distributed_sources
 add_executable(unit-local EXCLUDE_FROM_ALL ${unit-distributed_sources})
 add_dependencies(tests unit-local)
 
-target_compile_options(unit-local PRIVATE ${ARB_CXXOPT_ARCH})
+target_compile_options(unit-local PRIVATE ${ARB_CXX_FLAGS_TARGET_FULL})
 target_compile_definitions(unit-local PRIVATE TEST_LOCAL)
 target_link_libraries(unit-local PRIVATE gtest arbor arborenv arbor-sup arbor-private-headers ext-tinyopt)
 
@@ -19,7 +19,7 @@ if(ARB_WITH_MPI)
     add_executable(unit-mpi EXCLUDE_FROM_ALL ${unit-distributed_sources})
     add_dependencies(tests unit-mpi)
 
-    target_compile_options(unit-mpi PRIVATE ${ARB_CXXOPT_ARCH})
+    target_compile_options(unit-mpi PRIVATE ${ARB_CXX_FLAGS_TARGET_FULL})
     target_compile_definitions(unit-mpi PRIVATE TEST_MPI)
     target_link_libraries(unit-mpi PRIVATE gtest arbor arborenv arbor-sup arbor-private-headers ext-tinyopt)
 endif()
diff --git a/test/unit/CMakeLists.txt b/test/unit/CMakeLists.txt
index ff579936..b2a363df 100644
--- a/test/unit/CMakeLists.txt
+++ b/test/unit/CMakeLists.txt
@@ -215,6 +215,7 @@ if(${CMAKE_POSITION_INDEPENDENT_CODE})
     SOURCES "${CMAKE_CURRENT_SOURCE_DIR}/dummy"
     OUTPUT "CAT_DUMMY_SOURCES"
     MECHS dummy
+    CXX_FLAGS_TARGET ${ARB_CXX_FLAGS_TARGET_FULL}
     ARBOR "${PROJECT_SOURCE_DIR}"
     STANDALONE ON
     VERBOSE OFF)
@@ -236,7 +237,7 @@ if(ARB_WITH_HIP_CLANG)
     target_compile_options(unit PRIVATE $<$<COMPILE_LANGUAGE:CXX>:${clang_options_}>)
 endif()
 
-target_compile_options(unit PRIVATE ${ARB_CXXOPT_ARCH})
+target_compile_options(unit PRIVATE ${ARB_CXX_FLAGS_TARGET_FULL})
 target_compile_definitions(unit PRIVATE "-DDATADIR=\"${CMAKE_CURRENT_SOURCE_DIR}/../swc\"")
 target_compile_definitions(unit PRIVATE "-DLIBDIR=\"${PROJECT_BINARY_DIR}/lib\"")
 target_include_directories(unit PRIVATE "${CMAKE_CURRENT_BINARY_DIR}")
-- 
GitLab