Keeping all the checks in one place for future simplification.
Couple of minor points around loss of specific error information and we might want another DP instance but otherwise looks good. std::pair<hsa_status_t, bool> to hsa_status_t is an improvement imo, we don't need particularly fine grained understanding of what went wrong when the plan on failure is to log something and keep going.
return Err here, it might be more informative than HSA_STATUS_ERROR when printed to a log
i'd probably parenthesize this, (AllocAllowed && Size > 0)
better to preserve the specific error
somewhat inconsistent hat we aren't printing here. Maybe we want something like DP("Skipping memory pool: %s\n");
Unforeseen consequence arises here. Since isValidMemoryPool now returns false for zero size pools, a zero size pool will make addKernArgPool return an error, and that makes findKernargPool return an error, which aborts construction of the plugin. I have a system locally with two non-zero kernarg pools and to zero size ones which runs into this.
Going to patch by letting FindKernargPool keep looking on an error reported by the lower levels since a zero size pool is broadly equivalent to one we can't use for another reason.