diff options
| author | agozillon <Andrew.Gozillon@amd.com> | 2024-08-23 19:48:43 +0200 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-08-23 19:48:43 +0200 |
| commit | f4cf93fb509c53771d61a973f27be9b1a90dee0a (patch) | |
| tree | bf8e9323828675c92dc3fb20845d0a73c88c95b1 /offload/test/offloading | |
| parent | f607102a0d6be0e2aebc1bfaed2ed0a6ae020145 (diff) | |
[Flang][OpenMP] Align map clause generation and fix issue with non-shared allocations for assumed shape/size descriptor types (#97855)
This PR aims to unify the map argument generation behavior across both
the implicit capture (captured in a target region) and the explicit
capture (process map), currently the varPtr field of the MapInfo for the
same variable will be different depending on how it's captured. This PR
tries to align that across the generations of MapInfoOp in the OpenMP
lowering.
Currently, I have opted to utilise the rawInput (input memref to a HLFIR
DeclareInfoOp) as opposed to the addr field which includes more
information. The side affect of this is that we have to deal with
BoxTypes less often, which will result in simpler maps in these cases.
The negative side affect of this is that we don't have access to the
bounds information through the resulting value, however, I believe the
bounds information we require in our case is still appropriately stored
in the map bounds, and this seems to be the case from testing so far.
The other fix is for cases where we end up with a BoxType argument into
a function (certain assumed shape and sizes cases do this) that has no
fir.ref wrapping it. As we need the Box to be a reference type to
actually utilise the operation to access the base address stored inside
and create the correct mappings we currently generate an intermediate
allocation in these cases, and then store into it, and utilise this as
the map argument, as opposed to the original.
However, as we were not sharing the same intermediate allocation across
all of the maps for a variable, this resulted in errors in certain cases
when detatching/attatching the data e.g. via enter and exit. This PR
adjusts this for cases
Currently we only maintain tracking of all intermediate allocations for
the current function scope, as opposed to module. Primarily as the only
case I am aware of that this is required is in cases where we pass
certain types of arguments to functions (so I opted to minimize the
overhead of the pass for now). It could likely be extended to module
scope if required if we find other cases where it's applicable and
causing issues.
Diffstat (limited to 'offload/test/offloading')
| -rw-r--r-- | offload/test/offloading/fortran/local-descriptor-map-regress.f90 | 75 |
1 files changed, 75 insertions, 0 deletions
diff --git a/offload/test/offloading/fortran/local-descriptor-map-regress.f90 b/offload/test/offloading/fortran/local-descriptor-map-regress.f90 new file mode 100644 index 000000000000..5f628b8ad8c7 --- /dev/null +++ b/offload/test/offloading/fortran/local-descriptor-map-regress.f90 @@ -0,0 +1,75 @@ +! Small regression test that checks that we do not cause +! a runtime map error in cases where we are required to +! allocate a local variable for the fortran descriptor +! to store into and then load from it, done so by +! re-using the temporary local variable across all +! maps related to the mapped variable and associated +! local variable to make sure that each map does as +! it's intended to do with the original data. This +! prevents blobs of local descriptor data remaining +! attatched on device long after it's supposed to, +! which can cause weird map issues later in susbequent +! function invocations. However, it doesn't avoid a user +! shooting themselves in the foot by mapping data via enter +! and then not providing a corresponding exit. +! REQUIRES: flang, amdgpu + +! RUN: %libomptarget-compile-fortran-run-and-check-generic +subroutine launchOne(n1, n2, ret) + implicit none + real, intent(out) :: ret + real(4), dimension(n1,n2) :: sbuf31 + integer :: n1,n2 +!$omp target enter data map(alloc:sbuf31) + +!$omp target + sbuf31(2, 2) = 10 +!$omp end target + +!$omp target update from(sbuf31) + + ret = sbuf31(2, 2) + +!$omp target exit data map(delete:sbuf31) +end subroutine launchOne + +subroutine launchTwo(N, ret) + implicit none + real, intent(inout) :: ret + integer :: N + real(4), dimension(N) :: p + +!$omp target enter data map(to:p) + +!$omp target + p(8) = 20 +!$omp end target + +!$omp target update from(p) + +ret = ret + p(8) + +! intentional non-deletion, can trigger an illegal map +! issue in cases where the local map we store and load +! from for the variable is different across all maps. +! Not too sure why this is the thing that triggers the +! problem in general. It seems like it would be an issue +! made apparent with and without this statement commented, +! especially as the issue occurs with the enter and not the +! corresponding exit (from the runtime trace at least). +!!$omp target exit data map(delete:p) +end subroutine launchTwo + +program reproducer + implicit none + integer :: N = 10 + integer :: nr = 10, nt = 10 + real :: output = 0 + + call launchOne(nr, nt, output) + call launchTwo(N, output) + + print *, output +end program reproducer + +! CHECK: 30 |
