[Openmp-commits] [PATCH] D133053: [Libomptarget] Change device free routines to accept the allocation kind

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added a comment.

In D133053#3764314 <https://reviews.llvm.org/D133053#3764314>, @JonChesterfield wrote:

> In D133053#3764266 <https://reviews.llvm.org/D133053#3764266>, @tianshilei1992 wrote:
>
>> You can still use `_v2` or whatever in `libomptarget`. For `libomptarget`, there is no old interface then. Just keep the old one and all the logic for compatibility in the plugin. There is no confusion.
>
> Then we'd have to keep the spurious hashtable inserts in alloc, and consider fixing the pointer leak in the current implementation (the table this deletes grows without bound). I.e. to keep things working for people that we don't have reason to believe exist, the cuda plugin must remain slower than it could be.

FWIW, we don't have to fix issues for old interface. That's the whole point of deprecating the old interface.

> I can make my peace with that, but it looks like discarding this worthwhile patch, not applying it and taking the extra complexity without the benefit.

Of course we will not discard this patch. We just need to find the right way to do it. As long as we make agreement on compatibility, we are good to go. That's a single cut, which can save a lot future arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133053