Page MenuHomePhabricator

[OpenMP][DeviceRT] Remove eager allocation for dynamic schedule handling
AcceptedPublic

Authored by jdoerfert on Mar 15 2021, 8:32 PM.

Details

Summary

This removes roughly 20% of the memory allocated statically by the
runtime (1010592744 vs 801186888 bytes). It will make loops without
statically known static schedule more expensive (for the one time
allocation and free). It will make new data environments, e.g., nested
parallels, cheaper, as we don't state/reload the "loop data" anymore.
Overall, it's a trade-off which makes sure you only pay for what you
use.

A better solution would be to extend the _dispatch_ API such that we
can pass in "stack-allocations" to hold the data. Since that requires
more work this seemed like a good first step.

Probably needs more testing.

Diff Detail

Event Timeline

jdoerfert created this revision.Mar 15 2021, 8:32 PM
jdoerfert requested review of this revision.Mar 15 2021, 8:32 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2021, 8:32 PM
Herald added a subscriber: sstefan1. · View Herald Transcript
jdoerfert updated this revision to Diff 330871.Mar 15 2021, 8:43 PM

Move type *before* the template class

ye-luo accepted this revision.Mar 18 2021, 4:40 PM

Memory saving is real on my side as well 3074MB -> 2534MB. I have been using this for a few days no problems found so far.

This revision is now accepted and ready to land.Mar 18 2021, 4:40 PM

I failed to apply this to amd-stg-open to test, but may be able to run qmcpack against llvm main now. Will try that.

I failed to apply this to amd-stg-open to test, but may be able to run qmcpack against llvm main now. Will try that.

What turns out in your try?

I failed to apply this to amd-stg-open to test, but may be able to run qmcpack against llvm main now. Will try that.

What turns out in your try?

qmcpack fails to build with trunk llvm on amdgcn, before applying this patch. Work in progress...

I failed to apply this to amd-stg-open to test, but may be able to run qmcpack against llvm main now. Will try that.

What turns out in your try?

qmcpack fails to build with trunk llvm on amdgcn, before applying this patch. Work in progress...

It is OK to commit this patch to trunk now? It sounds that your testing is stuck with another set of issues.

I'm not blocking. It doesn't merge cleanly into rocm so it's hard to run against aomp's testing.

The patch was written for qmcpack so it's not surprising it works there. @jdoerfert have you tried other applications?

Adding Ron in case he can run this against the amd-stg-open branch, and as fore warning of merge conflicts if not.

The patch was written for qmcpack so it's not surprising it works there. @jdoerfert have you tried other applications?

I did run it with miniqmc only. I don't assume any user of a statically known static schedule to be affected by this. And I do not know of applications that don't match that.

@ronlieb @JonChesterfield Can you run it (=trunk + this) against your tests?