[flang-commits] [flang] [llvm] [Flang][OpenMP] Fix Fortran automap handling (PR #162501)

[via flang-commits]

agozillon wrote:

I'm happy with the compiler changes, but I think we should drop the runtime changes and keep the test XFAIL'd for the moment.

The runtime fix is trying to address a problem that will be resolved without a runtime change when swapping to attach mapping, which will happen in the near future if I can land the PR stack I have up finally and can advance it forward. 

I've done a bit of a deep dive into this and the runtime fix appears to be trying to address two things, the main thing is that we don't actually support declare target to upstream fully until the aforementioned PR stack lands, so what we're actually doing in this test currently is still using a kernel input argument instead of the actual declare target global that's generated by the compiler and should be used in it's place (PR stack addresses this and a few dozen other things). 

Currently we map allocatables as a form of record type/C struct, where we have the descriptor as a parent, and the base address as a pointer map member that points to some data, like the below: 

omptarget --> Entry  0: Base=0x00005e1e0039c0a0, Begin=0x00005e1e0039c0a0, Size=48, Type=0x0, Name=/home/agozillo/git/aomp22.0/llvm-project/offload/test/offloading/fortran/declare-target-automap.f90
omptarget --> Entry  1: Base=0x00005e1e0039c0a0, Begin=0x00005e1e0039c0a0, Size=48, Type=0x1000000000001, Name=/home/agozillo/git/aomp22.0/llvm-project/offload/test/offloading/fortran/declare-target-automap.f90
omptarget --> Entry  2: Base=0x00005e1e0039c0a0, Begin=0x00005e1dfedc8c90, Size=8, Type=0x1000000000001, Name=/home/agozillo/git/aomp22.0/llvm-project/offload/test/offloading/fortran/declare-target-automap.f90
omptarget --> Entry  3: Base=0x00005e1dfedc8c90, Begin=0x00005e1e0039c0a0, Size=40, Type=0x1000000000011, Name=/home/agozillo/git/aomp22.0/llvm-project/offload/test/offloading/fortran/declare-target-automap.f90

The 8-bytes is what the OMP_PTR_AND_OBJ flag gets applied to and eventually runs through the runtime changes we add in this PR. The change is effectively tricking the runtime into thinking the 8-byte mapping should map the same bits as Entry 1. 

Later the has_device_addr map we give to the compiler has the same address as entry 2's begin and entry 3's base address, which is likely confusing the runtime a little. It likely should have the same address as entry 0/1 (but this alone isn't going to fix the issue I think).

But I believe the main issue is we've made an example that depends on declare target to/enter, which is only partially working upstream at the moment and we're trying to fix an issue that can't be addressed correctly/fully yet. So my thoughts are we keep the test xfailed for a bit longer, and land the compiler changes and wait a bit for the rest of the support to land to revisit this (can revisit when the PR stack lands to see if it'll work with just that, but I've verified that this will work downstream with the compiler changes in this PR and the attach map work) :-) 


https://github.com/llvm/llvm-project/pull/162501