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

[Joseph Huber via Phabricator via Openmp-commits]

jhuber6 added a comment.

In D133053#3764298 <https://reviews.llvm.org/D133053#3764298>, @tianshilei1992 wrote:

> In D133053#3764277 <https://reviews.llvm.org/D133053#3764277>, @jhuber6 wrote:
>
>> In D133053#3764276 <https://reviews.llvm.org/D133053#3764276>, @tianshilei1992 wrote:
>>
>>> In D133053#3764270 <https://reviews.llvm.org/D133053#3764270>, @jhuber6 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.
>>>>
>>>> The potential change here is users who used `omp_target_free` to free non-device pointers as this worked before. We technically export the plugin routines as they need to be visible to `libomptarget` but I don't know if we expect any users to call them directly.
>>>
>>> The "user" here is old `libomptarget`.
>>
>> It would be an incredibly weird setup to have `libomptarget` load a plugin from a different install right? We install them to the same directory so it would mean that the user manually edited `libomptarget` after the install.
>
> We don't force the plugins to be loaded from the same dir as `libomptarget` right?
>
> Again, like I said, we can claim LLVM 15 is the last version to guarantee backward compatibility for all the components we require. For later version all components have to be from the same build. We just need to make a consensus in the meeting, document it, and do whatever we want to change the APIs in the future.

When we load the plugin we just pass the name like `libomptarget.rtl.cuda.so` so it goes to the linker's search path which is going to search using the default path. I suppose since we use `-rpath` it's possible these could get mixed up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133053