[Openmp-commits] [mlir] [flang] [openmp] [Flang][OpenMP] Initial mapping of Fortran pointers and allocatables for target devices (PR #71766)

[via Openmp-commits]

agozillon wrote:

Sorry for the ping, hoping for a little guidance and a nudge in the right direction if possible! I've ran into a little issue trying the method of containing members in a map_info, I aluded to the problem a bit in the larger post I made above and is part of the reason I opted for the original direction. 

For the OpenMP structure mapping that is utilised in this patch for mapping descriptors, it maps the structure (the descriptor) and then members (the pointers, just data for now), however, to "attach" the pointer to the structure, the structure needs to be the same structure the pointer is a part of. Which sounds fairly simple and might even be and I'm just missing something. However, if my original mapping contains the original symbol (descriptor) and a BoxAddrOp to the generated base address from gatherDataOperandAddrAndBounds (data pointer in the descriptor), I run into a little bit of an issue. During the lowering from FIR -> LLVM-IR the transformation process generates alloca operations for various box related operations and this creates a little bit of a disconnect between the structure (descriptor) and member (data pointer) as the descriptor mapping remains pointing towards the appropriate construct (or perhaps inappropriate, if it should shift to the new alloca) and the member unfortunately re-targets one of the generated allocas. This causes a bit of a problem with the lowering as we're now trying to map a member that is no longer actually a member of the structure that it's being mapped with, which results in an OpenMP runtime error. A little example excerpt below which hopefully demonstrates what I mean reasonably:

FIR: 
 ```
 func.func @_QMtestPfunc_arg(%arg0: !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>> {fir.bindc_name = "arg_alloc"}) {
    ...
    %1 = fir.load %arg0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
    ... 
    %5 = fir.box_addr %1 : (!fir.box<!fir.heap<!fir.array<?xi32>>>) -> !fir.heap<!fir.array<?xi32>>
    %6 = omp.map_info var_ptr(%5 : !fir.heap<!fir.array<?xi32>>, !fir.array<?xi32>) map_clauses(tofrom) capture(ByRef) bounds(%4) -> !fir.heap<!fir.array<?xi32>> {name = "arg_alloc"}
    %7 = omp.map_info var_ptr(%arg0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>, !fir.box<!fir.heap<!fir.array<?xi32>>>) members(%6 : !fir.heap<!fir.array<?xi32>>) map_clauses(tofrom) capture(ByRef) -> !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>> {name = "arg_alloc"}
```

Lowered LLVM-IR (alloca %3 is stored into by %arg0): 

```
  llvm.func @_QMtestPfunc_arg(%arg0: !llvm.ptr {fir.bindc_name = "arg_alloc"}) {
    ...
    %3 = llvm.alloca %2 x !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)> {alignment = 8 : i64} : (i32) -> !llvm.ptr
    ....
    %19 = llvm.getelementptr %3[0, 0] : (!llvm.ptr) -> !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>
    %20 = llvm.load %19 : !llvm.ptr -> !llvm.ptr
    %21 = omp.map_info var_ptr(%20 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) bounds(%18) -> !llvm.ptr {name = "arg_alloc"}
    %22 = omp.map_info var_ptr(%arg0 : !llvm.ptr, !llvm.struct<(ptr, i64, i32, i8, i8, i8, i8, array<1 x array<3 x i64>>)>) members(%21 : !llvm.ptr) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = "arg_alloc"}

```
I can see two possible choices that might work (but I'd really love to know if anyone can see any better ones, I'm open to all suggestions):

1) Create an optimisation pass that will reaffirm that all mapped members point to the same alloca/structure type as their parent (or the other way around) after lowering and retarget them if they do not. I don't know enough about the FIR lowering to be sure of the correctness of this kind of pass unfortunately, so any insight into something like this would be appreciated.
2) Use the original base address returned by gatherDataOperandAddrAndBounds without a BoxAddrOp applied to it, this results in lowering to the correct alloca/structure representing the descriptor linked to the pointer, the main issue is that this currently can't be placed in the var_ptr field as it's not an OpenMPPointerLikeType and we also cannot rebind the original symbol to this to keep IsolatedFromAbove semantics in the target operation as it causes a variety of issues in lowering from it not being a reasonable replacement type. So I think we'd need a seperate field on map_info to hold this (and we'd have it attached in the same map_info as the original symbol, SSA %7/%22 in the above exerts) or perhaps varPtrPtr is reasonable.

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