[Libomptarget] Use the nextgen plugins by default.

[jhuber6]

Summary

The next-gen plugins are complete drop-in replacements for the old
versions. We should strive to replace the old ones as quickly as
possible now that we have a viable alternative.

The only test failing is the prelock.cpp test as the support has not landed in
the next-gen plugins.

[JonChesterfield]

I think we should land this (sounds like we need to default a test to the old plugin as part of the commit, if our CI is running?) asap. We don't really have the test coverage or resources to reliably manage both new and old plugins over the long haul.

[jhuber6]

@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

[kevinsala]

In 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 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

[ye-luo]

In D142399#4074871, @kevinsala wrote:

In 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 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.

openmp/libomptarget/src/rtl.cpp

void RTLsTy::loadRTLs() {
   PM->TargetOffloadPolicy =
       (kmp_target_offload_kind_t)__kmpc_get_target_offload();
   if (PM->TargetOffloadPolicy == tgt_disabled) {
     return;
   }
 
   DP("Loading RTLs...\n");
 
-  BoolEnvar NextGenPlugins("LIBOMPTARGET_NEXTGEN_PLUGINS", false);
+  BoolEnvar NextGenPlugins("LIBOMPTARGET_NEXTGEN_PLUGINS", true);
 
   // Attempt to open all the plugins and, if they exist, check if the interface
   // is correct and if they are supporting any devices.
   for (const char *Name : RTLNames) {
     AllRTLs.emplace_back();
 
     RTLInfoTy &RTL = AllRTLs.back();