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

Introduce QMC_DISABLE_HIP_HOST_REGISTER. ON by default. #4674

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

ye-luo
Copy link
Contributor

@ye-luo ye-luo commented Jul 13, 2023

Proposed changes

Since rocm 5.6. hipHostRegister becomes terribly slow https://github.com/ye-luo/miniqmc/wiki/hip-hsa-issues.
Introduce QMC_DISABLE_HIP_HOST_REGISTER. which can be used to make hipHostRegister no-op.
I set it ON by default for now. Will go through a few nights of performance test runs.

My guess is that it makes runs slower because we lose more from not using pinned memory than slower hipHostRegister in close to production benchmarks.

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

  • Other (please describe):

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'

prckent
prckent previously approved these changes Jul 14, 2023
@prckent
Copy link
Contributor

prckent commented Jul 14, 2023

It will be interesting to see if this helps or hinders the full range of system sizes run in the nightlies.

@prckent
Copy link
Contributor

prckent commented Jul 14, 2023

Test this please

@ye-luo ye-luo force-pushed the introduce-QMC_HIP_HOST_REGISTER branch from 00f2e9c to 6c5d36f Compare July 14, 2023 19:39
@ye-luo ye-luo changed the title Introduce QMC_HIP_HOST_REGISTER off by default. Introduce QMC_DISABLE_HIP_HOST_REGISTER. ON by default. Jul 14, 2023
@ye-luo
Copy link
Contributor Author

ye-luo commented Jul 14, 2023

Test this please

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.

LGTM

@prckent prckent merged commit 1e365df into QMCPACK:develop Jul 14, 2023
43 checks passed
@ye-luo ye-luo deleted the introduce-QMC_HIP_HOST_REGISTER branch July 14, 2023 21:04
@prckent prckent mentioned this pull request Aug 18, 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.

None yet

2 participants