-
Notifications
You must be signed in to change notification settings - Fork 25
Add missing pointer map in test_declare_mapper_present.c #898
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add missing pointer map in test_declare_mapper_present.c #898
Conversation
The test was mapping a pointee array-section `s.data[0:N]`, without mapping the base-pointer `s.data` itself, and then expecting `s.data` to be accessible on the device. The fix is to map `s.data` as well.
| // the always, the mapper force the update of the declared mapper | ||
| // to be seen outside the "target data region" | ||
|
|
||
| #pragma omp declare mapper(default:myvec_t v) map(always, present, from: v, v.len, v.data[0:v.len]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could add map(present, alloc: v.data) on the mapper as well, but it's not necessary for correctness.
|
Out of curiosity, is this change specific to Do we now also need And would Thanks! |
Yes, this is applicable to all array-sections with pointer bases. For any int * s_array = (int *) malloc(N*sizeof(int));
#pragma omp target enter data map(to: s_array[0:s.len]) // needs map(alloc: s_array)
printf("%d\n", omp_target_is_present(&s_array, omp_get_default_device()); // Should print 0 when s_array is not mapped.
Yes, |
|
Note that for scalar pointers, there is an implicit assumed-size-array-map on int * s_array = (int *) malloc(N*sizeof(int));
#pragma omp target enter data map(to: s_array[0:s.len])
#pragma omp target // implies map(s_array[:]) (or s_array[0:0] as per OpenMP 5.2)
{
s_array[0] = 111; // This is OK
}
#pragma omp target map(present, alloc: s_array) // But this present-check should fail
{} |
|
Got it, thank you! |
|
Just to check,since for the "simpler" case ( |
|
@abhinavgaba , I remember having a discussion in the Accelerators sub-committee meeting regarding this. Can you link the spec issue here ? |
There is an implicit
The only issue I could find that's related is regarding pointer attachment with zero-length array-sections: https://github.com/OpenMP/spec/issues/3259. To be clear, this is not a recent behavior change in the spec. Even in 5.0, there was no implicit map of a pointer The only case a "pointer" and its "pointee" are mapped together from a single list-item is for "referencing variables", like Fortran allocatable/pointers and C++ references. e.g.: integer, pointer :: p(:)
...
!$omp target_enter_data map(p) ! Equivalent to map(ref_ptr_ptee: p) i.e. maps the pointer
! as well as the pointee
... int x;
int &xr = x;
#pragma omp target_enter_data map(xr) ! Maps the "ref_ptr" xr, and its "pointee" x |
The test was mapping a pointee array-section
s.data[0:N], without mapping the base-pointers.dataitself, and then expectings.datato be accessible on the device.The fix is to map
s.dataas well.