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

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added inline comments.


================
Comment at: openmp/libomptarget/plugins/amdgpu/src/rtl.cpp:636
 
     return HSA_STATUS_SUCCESS;
   }
----------------
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?



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109875/new/

https://reviews.llvm.org/D109875