Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for OpenMP compiler support #4422

Merged
merged 5 commits into from
Jan 29, 2023

Conversation

PhilipFackler
Copy link
Contributor

Proposed changes

Added find_package call and meaningful error message.
Closes #4372

What type(s) of changes does this code introduce?

  • Build related changes

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

Ubuntu 22.04

Checklist

  • Yes. This PR is up to date with current the current state of 'develop'
  • No. Code added or changed in the PR has been clang-formatted
  • No. This PR adds tests to cover any new code, or to catch a bug that is being fixed
  • No. Documentation has been added (if appropriate)

Added find_package call and meaningful error message.
CMakeLists.txt Outdated
find_package(OpenMP)
if(NOT OpenMP_FOUND)
message(FATAL_ERROR "No compiler support for OpenMP..."
"need to use -DQMC_OMP=OFF")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since OpenMP is a critical part of QMCPACK simulations I wonder if it's best to suggest finding compiler support for it. @prckent and @ye-luo thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will push an edit

Copy link
Contributor

@prckent prckent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you been able to test this? e.g. Does the fruit company's compiler result in this error?

Copy link
Contributor

@prckent prckent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested this code, but I think that this OpenMP availability test will need to be higher up. e.g. if the libstdc++ check has -fopenmp on the compile line, it will already have failed before we reach this point. The OpenMP test should be one of the first run.

CMakeLists.txt Outdated
# Check for OpenMP compiler support if requested
#---------------------------------------------------------
if(QMC_OMP)
find_package(OpenMP)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment before this line saying

# OpenMP package currently is only used to to verify compiler support. No feature of this package is been used and we customize OpenMP options directly in per vendor configurations.

@prckent
Copy link
Contributor

prckent commented Jan 29, 2023

This test needs to go before the Testlibstdc++ check, otherwise that can fail for the wrong reason. After the move, the current AppleClang 14.0.0.14000029 fails on the cxx17 check. According to https://en.cppreference.com/w/cpp/compiler_support/17 this is the correct result since memory resources are not yet implemented. Hopefully this will change shortly.

with QMC_OMP=ON or not specified:

-- QMCPACK adds -fstandalone-debug for Debug builds
-- Could NOT find OpenMP_C (missing: OpenMP_C_FLAGS OpenMP_C_LIB_NAMES) 
-- Could NOT find OpenMP_CXX (missing: OpenMP_CXX_FLAGS OpenMP_CXX_LIB_NAMES) 
-- Could NOT find OpenMP (missing: OpenMP_C_FOUND OpenMP_CXX_FOUND) 
CMake Error at CMakeLists.txt:301 (message):
  No compiler support for OpenMP found.  Switching to a compiler with OpenMP
  support is recommended.  Alternatively, you will need to run CMake
  configure with -DQMC_OMP=OFF


-- Configuring incomplete, errors occurred!

with QMC_OMP=OFF:

-- C++17 standard library support not found or incomplete
compiler is AppleClang 14.0.0.14000029
Compiler detected is <Clang> namely clang++ or a vendor variant (icpx, amdclang++, armclang++).
  If not using libcxx, ensure a GCC toolchain version equal or greater than 9.0 gets picked up. Check with '<Clang> -v'. Or use the --gcc-toolchain compiler option (added to both CMAKE_C_FLAGS and CMAKE_CXX_FLAGS) to point to a newer GCC installation.
  Output of test compile is in cpp17_compile_fail.txt
CMake Error at CMake/TestCxx17Library.cmake:55 (message):
  stopping
Call Stack (most recent call first):
  CMakeLists.txt:310 (include)


-- Configuring incomplete, errors occurred!

@ye-luo
Copy link
Contributor

ye-luo commented Jan 29, 2023

@prckent I moved the openmp check up. Could you verify if it works for you? If yes. Please proceed to merge this PR.

@prckent
Copy link
Contributor

prckent commented Jan 29, 2023

Can confirm your fix reproduces my local fix.

@prckent
Copy link
Contributor

prckent commented Jan 29, 2023

Test this please

@prckent prckent merged commit a0bbb67 into QMCPACK:develop Jan 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test for OpenMP compiler support
4 participants