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

Adjust scalar/vector math function handling #3357

Merged
merged 8 commits into from
Aug 13, 2021

Conversation

ye-luo
Copy link
Contributor

@ye-luo ye-luo commented Aug 13, 2021

Proposed changes

Part A:
sincos function is used both inside and outside openmp offload region.
When used inside target region, the generic call signature defined in glibc is expected.
We need to be sure they can be compiled and linked on both host and device.

On the other hand, vendor may provide optimized functions requires call redirection like amd_sincos.
If we use inline function, this pollutes the compilation of target region. For this reason, This PR splits math functions in two namespaces.

  1. qmcplusplus for CPU only.
  2. omptarget for being used inside target region.

Part B:
When doing the above work, I reviewed all the scalar vector math handling. To reduce entanglement, QMC_MATH_VENDOR is introduced with providers GENERIC, INTEL_VML, AMD_LIBM, IBM_MASS. ENABLE_MASS has been removed.

When MKL is detected in BLAS/LAPACK, INTEL_VML will be picked as in the past. But if GENERIC is requested, the use of VML can be skipped. In general, no change for users but we have more precise control.

Another note, I explored the AMD LibM but it is slower than Clang inlined functions. The status of vector functions are also not clear, amd/aocl-libm-ose#8 So it is added only for experiments and should be avoided for production use.

Part C:
Added a unit test for StructFact. it is very useful to quickly check sincos performance.

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

  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Testing changes
  • Documentation changes

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

epyc-server, dell-laptop, summit

Checklist

  • Yes. This PR is up to date with current the current state of 'develop'

@ye-luo ye-luo requested a review from PDoakORNL August 13, 2021 18:11
@ye-luo ye-luo changed the title Adjust Scalar/vector math function handling Adjust scalar/vector math function handling Aug 13, 2021
Copy link
Contributor

@PDoakORNL PDoakORNL left a comment

Choose a reason for hiding this comment

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

This looks good to me. The cmake improvements are welcome.

@ye-luo
Copy link
Contributor Author

ye-luo commented Aug 13, 2021

Start testing in-house

@ye-luo ye-luo merged commit af4fce1 into QMCPACK:develop Aug 13, 2021
@ye-luo ye-luo deleted the separate-omptarget-math branch August 13, 2021 19:38
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.

2 participants