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

Reflect HIP build options in printout #3919

Merged
merged 3 commits into from
Mar 28, 2022
Merged

Conversation

ye-luo
Copy link
Contributor

@ye-luo ye-luo commented Mar 25, 2022

Proposed changes

ENABLE_ROCM is a CMake option not a macro. It should not be used in the source codes.

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

  • Bugfix

Does this introduce a breaking change?

  • No

What systems has this change been tested on?

epyc-server

Checklist

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

ENABLE_ROCM is a CMake option not a macro. It should not be used in the source codes.
@ye-luo
Copy link
Contributor Author

ye-luo commented Mar 25, 2022

Test this please

src/QMCApp/QMCMain.cpp Outdated Show resolved Hide resolved
app_summary() << " CPU only build" << std::endl;
#else
#if defined(ENABLE_OFFLOAD)
app_summary() << " OpenMP target offload to accelerators build option is enabled" << std::endl;
#endif
#if defined(ENABLE_CUDA) || defined(QMC_CUDA)
#if defined(ENABLE_HIP) || defined(QMC_CUDA2HIP)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is probably out of scope for this PR.

We have too many hip options. If ENABLE_HIP is on QMC_CUDA2HIP must be on since you are by definition on a HIP system and if the CUDA is built it will be broken.
I don't see any good reason to allow QMC_CUDA2HIP and not ENABLE_HIP.

However as long as all those options exist and can be used in different combination they should probably give separate output. Otherwise you need to have access to the build options to really know what you are running anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is not true. ENABLE_HIP is direct HIP source code in AFQMC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated printout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@PDoakORNL how about the current printout?

@ye-luo
Copy link
Contributor Author

ye-luo commented Mar 28, 2022

Test this please

@ye-luo ye-luo merged commit cb79e3c into QMCPACK:develop Mar 28, 2022
@ye-luo ye-luo deleted the correct-macro branch March 28, 2022 16:58
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