[Openmp-commits] [PATCH] D142512: [OpenMP][libomptarget] Fix mapping/prelock.cpp test

[Carlo Bertolli via Phabricator via Openmp-commits]

carlo.bertolli added a comment.

In D142512#4079226 <https://reviews.llvm.org/D142512#4079226>, @kevinsala wrote:

> @carlo.bertolli Just tested the example below with the old plugins and it also segfaults:
>
>   #include <assert.h>
>   #include <stdio.h>
>   #include <stdlib.h>
>   #include <string.h>
>   
>   #include <omp.h>
>   
>   void *llvm_omp_target_lock_mem(void *ptr, size_t size, int device_num);
>   void llvm_omp_target_unlock_mem(void *ptr, int device_num);
>   
>   int main(int argc, char **argv) {
>       const size_t size = 1000*sizeof(int);
>   
>       int *ptr1 = (int *) malloc(size);
>       assert(ptr1 != NULL);
>   
>       memset(ptr1, 0, size);
>   
>       int *ptr2 = (int *) llvm_omp_target_lock_mem(ptr1, size, 0);
>       assert(ptr2 != NULL);
>   
>       fprintf(stdout, "ptr1: %p, ptr2: %p\n", ptr1, ptr2);
>       fprintf(stdout, "ptr1[0]: %d\n", ptr1[0]);
>       fprintf(stdout, "ptr2[0]: %d\n", ptr2[0]);
>   
>       return 0;
>   }
>
> When running the program, the access to `ptr2[0]` segfaults at the host side:
>
>   ptr1: 0x55b461f00ec0, ptr2: 0x7fc5576c8ec0
>   ptr1[0]: 0
>   Segmentation fault
>
> So I don't think `ptr2` is a locked host pointer. It's the pointer returned by `hsa_amd_memory_lock` as the `agent_ptr` output parameter. And probably it's a pointer that the GPU agents should use for asynchronous memory transfers involving the buffer `[ptr1, size)`. I assumed the host locked pointer is `ptr1` directly, after `hsa_amd_memory_lock` is executed. However, I'm not 100% sure since the documentation of the lock operation on HSA AMD is a bit confusing.

Agreed: a pointer returned by the lock function should not be used on the host. It can be used in map clause as it will be passed to the memory copy functions along with the device agent.
This test is different from prelock.cpp and I was not expecting this to work.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142512