[Openmp-commits] [PATCH] D109875: [AMDGPU][Libomptarget] Refactor device/host memory pools setup

Jon Chesterfield via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Mon Sep 20 03:42:10 PDT 2021

JonChesterfield added inline comments.

Comment at: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:636
pdhaliwal wrote:
> JonChesterfield wrote:
> > pdhaliwal wrote:
> > > JonChesterfield wrote:
> > > > This is going to return success if it finds a pool for either kernarg or fine grain. I think we should only be claiming success if we've found (at least?) one for each. We could handle the control flow for that by initialising both pools to {0} and checking neither is {0} before returning, or by keeping track of whether both have been initialized
> > > Thanks for pointing this out. It actually found an issue when only one memory pool is available and it is both kernarg and fine grained. 
> > That's interesting. If we have only one pool for both then we must use it for both.
> > 
> > What if we have one pool for both and one for only fine grain? Presumably we should then use different ones as they may be backed by different allocators.
> > 
> > Iteration order is probably difficult to predict. Maybe first kernarg pool found is used, and first non-kernarg pool is used for fine grain, but before erroring out we also use the kernarg pool for fine grain iff it hasn't been assigned yet?
> > 
> That makes sense. Using "if-else" instead of "only if" can make sure that in each iteration only one of the kernarg and fine grained pool is assigned. After the for loop, kernarg should be always assigned (I guess?). If `FineGrainedMemoryPool` is not set then kernarg pool can be reused. Issues/Suggestions for this approach?
Sounds good to me

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list