[Openmp-commits] [PATCH] D81054: [OpenMP] Introduce target memory manager

Shilei Tian via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Tue Aug 11 13:36:55 PDT 2020

tianshilei1992 added a comment.

In D81054#2208730 <https://reviews.llvm.org/D81054#2208730>, @jdoerfert wrote:

> Some comments and nits you should take under consideration.
> I'm not 100% sold on the list design, that we look for the exact size, and that we traverse the list while we look.

The "list" is not a real list. It is a `std::multiset` here. So basically its look up complexity is `O(logn)` on average. If we don't have such a thing, what would be a better way to organize those free nodes with different sizes?

> I'd rename `memory.h` into `MemoryManager.h` if we don't expect anything else to go in there that is not "a memory manager" at the end of the day. Same with the cpp.


> I'm not sure we need the `memory` namespace, or the `impl` namespace for that matter.

I prefer to leave the namespace. Current implementation of `libomptarget` has really poor code style. This is a totally new file. I hope to make it right from it.

  rG LLVM Github Monorepo



More information about the Openmp-commits mailing list