Skip to content

Commit

Permalink
Merge pull request #682 from LLNL/bugfix/white238/openmp
Browse files Browse the repository at this point in the history
OpenMP and policy warning clean up
  • Loading branch information
white238 authored Mar 7, 2024
2 parents 148c53e + ee7dc76 commit 9c59df4
Show file tree
Hide file tree
Showing 9 changed files with 145 additions and 187 deletions.
2 changes: 1 addition & 1 deletion .gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ stages:
.build_script:
script:
#Use pre-existing allocation if any
- JOBID=$(if [[ $SYS_TYPE = toss_* ]]; then squeue -h --name=${PROJECT_ALLOC_NAME} --format=%A; fi)
- JOBID=$(if [[ "$SYS_TYPE" == "toss_4_x86_64_ib" ]]; then squeue -h --name=${PROJECT_ALLOC_NAME} --format=%A; fi)
- ASSIGN_ID=$(if [[ -n "${JOBID}" ]]; then echo "--jobid=${JOBID}"; fi)
#BUILD + TEST
- echo -e "section_start:$(date +%s):build_and_test\r\e[0K
Expand Down
12 changes: 12 additions & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@ The project release numbers follow [Semantic Versioning](http://semver.org/spec/

## [Unreleased] - Release date yyyy-mm-dd

### Added
- Added output for CMake's implicitly added link libraries and directories.

### Changed
- OpenMP target now uses a generator expression for Fortran flags instead of replacing flags in
Fortran targets created with BLT macros.
- Remove setting CMP0076 to OLD which left relative paths in `target_sources()` instead of altering
them to absolute paths.
- Header-only libraries headers now show up under their own target in IDE project views instead of
under downstream targets. This only works in CMake >= 3.19, otherwise they will not show up at all.
- Raised version for base CMake version supported by BLT to 3.15 to support ALIAS targets across BLT.

## [Version 0.6.1] - Release date 2024-01-29

### Fixed
Expand Down
17 changes: 8 additions & 9 deletions SetupBLT.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ if (NOT BLT_LOADED)
endif()
unset(_is_multi_config)

if(${CMAKE_VERSION} VERSION_LESS 3.8.0)
if(${CMAKE_VERSION} VERSION_LESS 3.15.0)
message("*************************************")
message("* Unsupported version of CMake detected.")
message("* BLT requires CMake 3.8 or above.")
message("* BLT requires CMake 3.15 or above.")
message("* Some BLT features may not work.")
message("*************************************")
endif()
Expand Down Expand Up @@ -79,13 +79,12 @@ if (NOT BLT_LOADED)
cmake_policy(SET CMP0074 NEW)
endif()

# New turns relative target_sources() paths to absolute
# Policy added in 3.13+
# NOTE: this will be deprecated eventually but NEW causes
# problems in header only libraries, OLD keeps current behavior
if(POLICY CMP0076)
cmake_policy(SET CMP0076 OLD)
endif()
################################
# Enable various find commands to look in non-default paths
################################
set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB32_PATHS TRUE)
set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIB64_PATHS TRUE)
set_property(GLOBAL PROPERTY FIND_LIBRARY_USE_LIBX32_PATHS TRUE)

################################
# Invoke CMake Fortran setup
Expand Down
26 changes: 7 additions & 19 deletions cmake/BLTMacros.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -209,19 +209,13 @@ macro(blt_add_library)
#
# Header-only library support
#
foreach (_file ${arg_HEADERS})
# Determine build location of headers
get_filename_component(_absolute ${_file} ABSOLUTE)
list(APPEND _build_headers ${_absolute})
endforeach()

#Note: This only works if both libraries are handled in the same directory,
# otherwise just don't include non-header files in your source list.
set_source_files_properties(${_build_headers} PROPERTIES HEADER_FILE_ONLY ON)

add_library( ${arg_NAME} INTERFACE )
target_sources( ${arg_NAME} INTERFACE
$<BUILD_INTERFACE:${_build_headers}>)
if( ${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.19.0" )
# Adding headers here allows them to show up in IDE projects but is not
# necessary for building
add_library( ${arg_NAME} INTERFACE ${arg_HEADERS} )
else()
add_library( ${arg_NAME} INTERFACE )
endif()
endif()

# Clear value of _have_fortran from previous calls
Expand Down Expand Up @@ -348,12 +342,6 @@ macro(blt_add_executable)
set_target_properties(${arg_NAME} PROPERTIES LINKER_LANGUAGE ${_blt_link_lang})
endif()

# fix the openmp flags for fortran if needed
# NOTE: this needs to be called after blt_setup_target()
if (_lang STREQUAL Fortran)
blt_fix_fortran_openmp_flags( ${arg_NAME} )
endif()

if ( arg_INCLUDES )
target_include_directories(${arg_NAME} PUBLIC ${arg_INCLUDES})
endif()
Expand Down
77 changes: 0 additions & 77 deletions cmake/BLTPrivateMacros.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -20,83 +20,6 @@ function(blt_error_if_target_exists target_name error_msg)
endif()
endfunction()

##-----------------------------------------------------------------------------
## blt_fix_fortran_openmp_flags(<target name>)
##
## Fixes the openmp flags for a Fortran target if they are different from the
## corresponding C/C++ OpenMP flags.
##-----------------------------------------------------------------------------
function(blt_fix_fortran_openmp_flags target_name)

if (ENABLE_FORTRAN AND ENABLE_OPENMP AND BLT_OPENMP_FLAGS_DIFFER)

# The OpenMP interface library will have been added as a direct
# link dependency instead of via flags
get_target_property(target_link_libs ${target_name} LINK_LIBRARIES)
if ( target_link_libs )
# Since this is only called on executable targets we can safely convert
# from a "real" target back to a "fake" one as this is a sink vertex in
# the DAG. Only the link flags need to be modified.
list(FIND target_link_libs "openmp" _omp_index)
list(FIND target_link_libs "blt::openmp" _blt_omp_index)
if(${_omp_index} GREATER -1 OR ${_blt_omp_index} GREATER -1)
message(STATUS "Fixing OpenMP Flags for target[${target_name}]")

# Remove openmp from libraries
list(REMOVE_ITEM target_link_libs "openmp")
list(REMOVE_ITEM target_link_libs "blt::openmp")
set_target_properties( ${target_name} PROPERTIES
LINK_LIBRARIES "${target_link_libs}" )

# Add openmp compile flags verbatim w/ generator expression
get_target_property(omp_compile_flags openmp INTERFACE_COMPILE_OPTIONS)
target_compile_options(${target_name} PUBLIC ${omp_compile_flags})

# Change CXX flags to Fortran flags

# These are set through blt_add_target_link_flags which needs to use
# the link_libraries for interface libraries in CMake < 3.13
if( ${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.13.0" )
get_target_property(omp_link_flags openmp INTERFACE_LINK_OPTIONS)
else()
get_target_property(omp_link_flags openmp INTERFACE_LINK_LIBRARIES)
endif()

string( REPLACE "${OpenMP_CXX_FLAGS}" "${OpenMP_Fortran_FLAGS}"
correct_omp_link_flags
"${omp_link_flags}"
)
if( ${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.13.0" )
target_link_options(${target_name} PRIVATE "${correct_omp_link_flags}")
else()
set_property(TARGET ${target_name} APPEND PROPERTY LINK_FLAGS "${correct_omp_link_flags}")
endif()
endif()

# Handle registered library general case

# OpenMP is an interface library which doesn't have a LINK_FLAGS property
# in versions < 3.13
set(_property_name LINK_FLAGS)
if( ${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.13.0" )
# In CMake 3.13+, LINK_FLAGS was converted to LINK_OPTIONS.
set(_property_name LINK_OPTIONS)
endif()
get_target_property(target_link_flags ${target_name} ${_property_name})
if ( target_link_flags )

string( REPLACE "${OpenMP_CXX_FLAGS}" "${OpenMP_Fortran_FLAGS}"
correct_link_flags
"${target_link_flags}"
)
set_target_properties( ${target_name} PROPERTIES ${_property_name}
"${correct_link_flags}" )
endif()
endif()

endif()

endfunction()

##-----------------------------------------------------------------------------
## blt_find_executable(NAME <name of program to find>
Expand Down
52 changes: 30 additions & 22 deletions cmake/BLTSetupTargets.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
#
# SPDX-License-Identifier: (BSD-3-Clause)

# This file is intended to be included in the *-config.cmake files of
# any project using a third-party library. The macro
# `blt_install_tpl_setups(DESTINATION <dir>)` installs this file
# This file is intended to be included in the installed config files of
# any project using BLT's third-party library as well as in a BLT project.
# The macro `blt_install_tpl_setups(DESTINATION <dir>)` installs this file
# into the destination specified by the argument <dir>.

# BLTInstallableMacros provides helper macros for setting up and creating
Expand All @@ -14,40 +14,48 @@
if (NOT BLT_LOADED)
include("${CMAKE_CURRENT_LIST_DIR}/BLTInstallableMacros.cmake")
endif()
# If the generated TPL config file exists, include it here.

# Handle the two cases of TPL config variables, installed from upstream project
# and the current/main BLT project. Prefix all variables here to not conflict with
# non-BLT projects that load this as a configuration file.
if (EXISTS "${CMAKE_CURRENT_LIST_DIR}/BLTThirdPartyConfigFlags.cmake")
# Case: Imported BLT project (ie. an installed TPL loading its BLT targets)
include("${CMAKE_CURRENT_LIST_DIR}/BLTThirdPartyConfigFlags.cmake")
else()
# Otherwise, configure the TPL flags. We have to prefix these variables with
# BLT so that they never conflict with user-level CMake variables in downstream
# projects.
#
# We have to do some checks before we overwrite these internal variables to assure
# that we don't turn off a third-party library enabled by the dependency.
if (NOT DEFINED BLT_ENABLE_HIP OR NOT ${BLT_ENABLE_HIP})
# Case: Main BLT project (ie. a project loading it's own BLT)
#
# Always stay enabled if any upstream has already turned you on.
if(NOT BLT_ENABLE_HIP)
set(BLT_ENABLE_HIP ${ENABLE_HIP})
endif()
if (NOT DEFINED BLT_ENABLE_CUDA OR NOT ${BLT_ENABLE_CUDA})
if(NOT BLT_ENABLE_CUDA)
set(BLT_ENABLE_CUDA ${ENABLE_CUDA})
endif()
if (NOT DEFINED BLT_ENABLE_MPI OR NOT ${BLT_ENABLE_MPI})
if(NOT BLT_ENABLE_MPI)
set(BLT_ENABLE_MPI ${ENABLE_MPI})
endif()
if (NOT DEFINED BLT_ENABLE_OPENMP OR NOT ${BLT_ENABLE_OPENMP})
if(NOT BLT_ENABLE_OPENMP)
set(BLT_ENABLE_OPENMP ${ENABLE_OPENMP})
endif()
if (NOT DEFINED BLT_ENABLE_FIND_MPI OR NOT ${BLT_ENABLE_FIND_MPI})
if(NOT BLT_ENABLE_FIND_MPI)
set(BLT_ENABLE_FIND_MPI ${ENABLE_FIND_MPI})
endif()
set(BLT_ENABLE_CLANG_CUDA ${ENABLE_CLANG_CUDA})
if(NOT BLT_ENABLE_CLANG_CUDA)
set(BLT_ENABLE_CLANG_CUDA ${ENABLE_CLANG_CUDA})
endif()

message(STATUS "BLT MPI support is ${BLT_ENABLE_MPI}")
message(STATUS "BLT OpenMP support is ${BLT_ENABLE_OPENMP}")
message(STATUS "BLT CUDA support is ${BLT_ENABLE_CUDA}")
message(STATUS "BLT HIP support is ${BLT_ENABLE_HIP}")
endif()

# Detect if Fortran has been introduced.
get_property(_languages GLOBAL PROPERTY ENABLED_LANGUAGES)
if(_languages MATCHES "Fortran")
set(_fortran_already_enabled TRUE)
set(_fortran_already_enabled TRUE)
else()
set(_fortran_already_enabled FALSE)
set(_fortran_already_enabled FALSE)
endif()

# Only update ENABLE_FORTRAN if it is a new requirement, don't turn
Expand All @@ -62,8 +70,8 @@ endif()
# MPI
#------------------------------------
if (NOT TARGET mpi)
message(STATUS "MPI Support is ${BLT_ENABLE_MPI}")
if (BLT_ENABLE_MPI AND EXISTS "${CMAKE_CURRENT_LIST_DIR}/thirdparty/BLTSetupMPI.cmake")
message(STATUS "Creating BLT MPI targets...")
include("${CMAKE_CURRENT_LIST_DIR}/thirdparty/BLTSetupMPI.cmake")
endif()
endif()
Expand All @@ -73,8 +81,8 @@ endif()
# OpenMP
#------------------------------------
if (NOT TARGET openmp)
message(STATUS "OpenMP Support is ${BLT_ENABLE_OPENMP}")
if (BLT_ENABLE_OPENMP AND EXISTS "${CMAKE_CURRENT_LIST_DIR}/thirdparty/BLTSetupOpenMP.cmake")
message(STATUS "Creating BLT OpenMP targets...")
include("${CMAKE_CURRENT_LIST_DIR}/thirdparty/BLTSetupOpenMP.cmake")
endif()
endif()
Expand All @@ -84,8 +92,8 @@ endif()
# CUDA
#------------------------------------
if (NOT TARGET cuda)
message(STATUS "CUDA Support is ${BLT_ENABLE_CUDA}")
if (BLT_ENABLE_CUDA AND EXISTS "${CMAKE_CURRENT_LIST_DIR}/thirdparty/BLTSetupCUDA.cmake")
message(STATUS "Creating BLT CUDA targets...")
include("${CMAKE_CURRENT_LIST_DIR}/thirdparty/BLTSetupCUDA.cmake")
endif()
endif()
Expand All @@ -95,8 +103,8 @@ endif()
# HIP
#------------------------------------
if (NOT TARGET blt_hip)
message(STATUS "HIP Support is ${BLT_ENABLE_HIP}")
if (BLT_ENABLE_HIP AND EXISTS "${CMAKE_CURRENT_LIST_DIR}/thirdparty/BLTSetupHIP.cmake")
message(STATUS "Creating BLT HIP targets...")
include("${CMAKE_CURRENT_LIST_DIR}/thirdparty/BLTSetupHIP.cmake")
endif()
endif()
43 changes: 19 additions & 24 deletions cmake/BLTThirdPartyConfigFlags.cmake.in
Original file line number Diff line number Diff line change
@@ -1,40 +1,35 @@
if ("@ENABLE_CUDA@")
set(BLT_ENABLE_CUDA "@ENABLE_CUDA@")
# Always stay enabled if any upstream has already turned you on.
if(NOT BLT_ENABLE_CUDA)
set(BLT_ENABLE_CUDA @ENABLE_CUDA@)
endif()

if ("@ENABLE_HIP@")
set(BLT_ENABLE_HIP "@ENABLE_HIP@")
if(NOT BLT_ENABLE_HIP)
set(BLT_ENABLE_HIP @ENABLE_HIP@)
endif()

if ("@ENABLE_MPI@")
set(BLT_ENABLE_MPI "@ENABLE_MPI@")
if(NOT BLT_ENABLE_MPI)
set(BLT_ENABLE_MPI @ENABLE_MPI@)
endif()

if ("@ENABLE_OPENMP@")
set(BLT_ENABLE_OPENMP "@ENABLE_OPENMP@")
if(NOT BLT_ENABLE_OPENMP)
set(BLT_ENABLE_OPENMP @ENABLE_OPENMP@)
endif()
if(NOT BLT_ENABLE_FIND_MPI)
set(BLT_ENABLE_FIND_MPI @ENABLE_FIND_MPI@)
endif()

set(BLT_ENABLE_FIND_MPI "@ENABLE_FIND_MPI@")

# User configurable CUDA options needed by downstream. Normally, exporting
# these targets would pass these flags to downstream targets, but this
# must be done manually if targets aren't exported.
if ("@ENABLE_CLANG_CUDA@" AND NOT DEFINED ENABLE_CLANG_CUDA)
set(BLT_ENABLE_CLANG_CUDA "@ENABLE_CLANG_CUDA@")
# If the user specifies this variable, ignore the inherited flag.
elseif (DEFINED ENABLE_CLANG_CUDA)
# Always prefer the current project if they turned it on
if(ENABLE_CLANG_CUDA)
set(BLT_ENABLE_CLANG_CUDA ${ENABLE_CLANG_CUDA})
else()
set(BLT_ENABLE_CLANG_CUDA @ENABLE_CLANG_CUDA@)
endif()

if ("@BLT_CLANG_CUDA_ARCH@" AND NOT DEFINED BLT_CLANG_CUDA_ARCH)
if (NOT DEFINED BLT_CLANG_CUDA_ARCH)
set(BLT_CLANG_CUDA_ARCH "@BLT_CLANG_CUDA_ARCH@")
endif()

if ("@CUDA_TOOLKIT_ROOT_DIR@" AND NOT DEFINED CUDA_TOOLKIT_ROOT_DIR)
if (NOT DEFINED CUDA_TOOLKIT_ROOT_DIR)
set(CUDA_TOOLKIT_ROOT_DIR "@CUDA_TOOLKIT_ROOT_DIR@")
endif()

# User configurable HIP options needed by downstream
if ("@ROCM_PATH@" AND NOT DEFINED ROCM_PATH)
if (NOT DEFINED ROCM_PATH)
set(ROCM_PATH "@ROCM_PATH@")
endif()
Loading

0 comments on commit 9c59df4

Please sign in to comment.