Hello Kwok, hi all,
Kwok Cheung Yeung wrote:
Currently, GCC accepts an allocate clause (to use a specific memory
allocator and alignment) on the OpenMP target construct, but it has no
effect - memory is always allocated with the defaults.
This patch causes memory for privatized variables (i.e. variables in
private and firstprivate clauses) to be allocated with the specified
allocator and alignment in a similar fashion to how it is done for
parallel constructs, reusing the lower_private_allocate function.
Some remarks (to bystanders - but also related to the review):
* The code does not handle Fortran deep mapping, but that's not surprising
as that is not yet supported in general. See PR119905 for tracking.
* There is some room for improvement especially for 'firstprivate'. For
instance, for host fallback (or USM, at least when not page migrated?)
the original data does not need to be copied if it is copied again in
the target region.
And for the omp_default_mem_alloc allocator with alignment, the
alignment could also be done via the alignment stored in the 'kinds'
argument for firstprivate (and no local code) or DECL_ALIGN for 'private'.
Side remark: size_t align = (size_t) 1 << (kinds[i] >> 8);
looks very fragile on 32bit systems ... (We don't support 32bit
offload targets - and there are additional 64bit assumptions.)
I wonder how often the alignment-only issue will show up in real-world
code; it becomes most interesting with dynamic data, but there
'firstprivate' is only rarely used. For stack/static variables, the
alignment is carried over from the declaration (via the kinds or
DECL_ALIGN). Thus, there shouldn't be a real need for it.
→ I think we can leave the current implementation as is.
* A user-defined allocator is only permitted when defined through
'uses_allocators'; there is ongoing work to fix the old patch for
review comments and specification updated (including post-6.0).
But until that has landed, it cannot be used.
* * *
Can you also update libgomp.texi?
First the sectionhttps://gcc.gnu.org/onlinedocs/libgomp/OpenMP-5_002e0.html,
namely:
"allocate clause" — P — "Clause has no effect on target (PR113436)"
is then 'Y' as the bug is fixed.
Secondly, I wonder whether it makes sense to update
https://gcc.gnu.org/onlinedocs/libgomp/Memory-allocation.html
The main usage is covered by the second bullet point, but maybe we should add a
note
that it also works on 'target' - and, possibly that for firstprivatized data,
there
is first a copy to the device and then to the allocated memory.
* * *
The following testcase that uses a VLA fails with an ICE:
internal compiler error: in force_constant_size, at gimplify.cc:809
use omp_lib
implicit none (type, external)
integer :: s = 7
block
integer :: i(s)
!$omp target firstprivate(i) allocate(allocator(omp_low_lat_mem_alloc) : i)
block
integer :: a(1)
a(i(1)) = 4
end block
end block
end
* * *
Tobias,
who still has still to finish looking through the patch.
