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

Relax scratch space limits for HIP reductions #6029

Merged
merged 2 commits into from
Apr 3, 2023

Conversation

skyreflectedinmirrors
Copy link
Contributor

@skyreflectedinmirrors skyreflectedinmirrors commented Mar 31, 2023

This change is motivated by profiling of some LJ reduction kernels in LAMMPS. Essentially we noticed that in some cases, we ended up severely occupancy limited by the "gridDim <= blockDim" restriction. Omniperf showed very low L1/L2 utilization, but the limiter was simply that we were not launching enough blocks.

This is a simple change to relax the scratch space limit slightly (from blockDim to 4096) to increase occupancy, but still keep the scratch limits well bounded. I was not particularly scientific about choosing the new limit, and I'm definitely open to better heuristics, but this increased RHODO (4^3) perf in LAMMPS by ~28% (the key kernel is a ParReduce)

Change-Id: I66c73cb3e53b99d8d0b59f423ae77263ca7e176f
@skyreflectedinmirrors
Copy link
Contributor Author

cc: @Rombur / @dalg24 -- the one we discussed on slack

Change-Id: I1400b8e9bc8c0a5af237283e7fc4db641d05b3f8
@stanmoore1
Copy link
Contributor

@arghdos nice catch!

@dalg24
Copy link
Member

dalg24 commented Apr 3, 2023

I re-ran exercise 4 from the tutorials. Still not the nice plateau we'd like to see but it does look better with this PR.
arghdos_relax_scratch_limits

@skyreflectedinmirrors
Copy link
Contributor Author

That was next on my list of things to check :), I'll re-run my profiling on the new (smaller) dip and see if we can iron it out further

@dalg24
Copy link
Member

dalg24 commented Apr 3, 2023

Ignoring clearly unrelated GCC-8.4.0 failure

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

4 participants