[Openmp-commits] [PATCH] D142399: [Libomptarget] Use the nextgen plugins by default.

Ye Luo via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Mon Jan 23 14:02:02 PST 2023


ye-luo added a comment.

In D142399#4074871 <https://reviews.llvm.org/D142399#4074871>, @kevinsala wrote:

> In D142399#4074765 <https://reviews.llvm.org/D142399#4074765>, @jhuber6 wrote:
>
>> @carlo.bertolli @kevinsala The only tests the nextgen plugins fail for me are the following. Any update on getting this resolved? I think we should aim to land this before the fork and then work on deleting the old plugins after.
>>
>>   Failed Tests (2):
>>     libomptarget :: amdgcn-amd-amdhsa :: mapping/prelock.cpp
>>     libomptarget :: amdgcn-amd-amdhsa-LTO :: mapping/prelock.cpp
>
> The patch D141227 <https://reviews.llvm.org/D141227> implements the lock/unlock API. Yesterday I updated the patch with new features. We can merge it if you all find it OK.
>
> Regarding the tests, even with the previous patch applied, they fail in the code line below. Is that line actually valid? We are trying to map an address (`locked`) that cannot be accessed by the host. That pointer is the `agent_ptr` returned by `hsa_amd_memory_lock`. If the plugin tries to access that buffer it will segfault. That's what is probably happening in the nextgen plugin because it tries to call `std::memcpy` on that "host pointer" in `__tgt_rtl_data_submit`.
>
> https://github.com/llvm/llvm-project/blob/main/openmp/libomptarget/test/mapping/prelock.cpp#L39

I agree with with Kevin. The test needs to be changed. It is not valid to map that "locked" ptr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142399



More information about the Openmp-commits mailing list