[clang] [llvm] [Clang][OpenMP] Capture mapped pointers on `target` by reference. (PR #145454)

[Abhinav Gaba via cfe-commits]

================
@@ -8811,8 +8829,19 @@ class MappableExprsHandler {
         ++EI;
       }
     }
-    llvm::stable_sort(DeclComponentLists, [](const MapData &LHS,
-                                             const MapData &RHS) {
+    llvm::stable_sort(DeclComponentLists, [VD](const MapData &LHS,
+                                               const MapData &RHS) {
+      // For cases like map(p, p[0], p[0][0]), the shortest map, like map(p)
----------------
abhinavgaba wrote:

Looks like this is not sufficient for cases like `map(sp, sp->x)`. In that case, the problem is that sp->x populates PartialStruct in https://github.com/llvm/llvm-project/blob/90f3147424a36652dae51aa690d9948117f8d906/clang/lib/CodeGen/CGOpenMPRuntime.cpp#L8985-L8990,

And even though initially after going through generateInfoFroCapture, `map(sp)` occupies the top of the map-chain in CurInfo:

```
&sp, &sp, sizeof(sp), TO | PARAM
&sp[0], &sp->x, sizeof(sp->x), TO
```

After, emitCombinedEntry, a new entry gets added for:
```
&sp[0], &sp[0], sizeof(sp[0]), ALLOC | PARAM
```
and all existing entries in CurInfo get the MEMBER_OF(1) bit applied to them.

https://github.com/llvm/llvm-project/blob/90f3147424a36652dae51aa690d9948117f8d906/clang/lib/CodeGen/CGOpenMPRuntime.cpp#L9516

So the final CombinedInfo after emitCombinedEntry and then appending CurInfo to it becomes:

```
&sp[0], &sp[0], sizeof(sp[0]), ALLOC | PARAM
&sp, &sp, sizeof(sp), TO | MEMBER_OF(1)
&sp[0], &sp->x, sizeof(sp->x), TO | MEMBER_OF(1)
```

instead of the desired:

```
&sp, &sp, sizeof(sp), TO | PARAM
&sp[0], &sp[0], sizeof(sp[0]), ALLOC
&sp[0], &sp->x, sizeof(sp->x), TO | MEMBER_OF(2)
```

(Note that this &sp getting MEMBER_OF(1) issue currently exists on target enter data as well.)

Maybe we can split up https://github.com/llvm/llvm-project/blob/90f3147424a36652dae51aa690d9948117f8d906/clang/lib/CodeGen/CGOpenMPRuntime.cpp#L9489-L9491 to first work on is_device_ptr, has_device_addr, and maps on pointers with component-lists of size one, and then subsequently on the remaining component-lists, but I'm not sure if I'm thinking of all the cases there, and if that'll cause issues.

```cpp
// Pseudo code-flow
// Directly pass the original CombinedInfo to the first two
generateInfoForCaptureForIDP/HDA(...&CombinedInfo...)
generateInfoForCaptureForMaps(&CombinedInfo..., /*filter for only component-lists that are single-length and for pointer base decls*/)

// Pass CurInfo to the next two
generateInfoForCaptureForMaps(&CurInfo, &PartialStruct, /*Filter for remining component-lists*/)
emitCombinedEntry(...&CurInfo, &PartialStruct...)
CombinedInfo.append(CurInfo)
```

@alexey-bataev, do you have any suggestions/feedback?

-----
Future Issue:

The problem long-term is that we currently assume that all component-lists for the same VAR should contribute towards a single combined PartialStruct, if any. That should not be the case for cases like:


```
map(to: sp->x, sp->y) map(to: sp->sq->a, sp->sq->b) map(sp)
```

In the above case, we should get two independent "containing-structs" mapped by themselves:

```
&sp, &sp, sizeof(sp), TO | PARAM

// map-chain for the containing-struct sp[0] with base-pointer sp
&sp[0], &sp[0], sizeof(sp[0]), ALLOC /* Can be optimized to not alloc the full struct */
&sp[0], &sp->x, sizeof(sp->x), TO | MEMBER_OF(2)
&sp[0], &sp->y, sizeof(sp->y), TO | MEMBER_OF(2)
&sp, &sp[0], ATTACH

// map-chain for the containing-struct sp->sq[0] with base-pointer sp->sq
&(sp->sq[0]), &(sp->sq[0]), sizeof(sp->sq[0]), ALLOC
&sp->sq[0], &sp->sq->a, sizeof(sp->sq->a), TO | MEMBER_OF(6)
&sp->sq[0], &sp->sq->a, sizeof(sp->sq->a), TO | MEMBER_OF(6)
&sp->sq, &sp->sq[0], ATTACH
```

So we need to loop-over all component-lists, and only process those together that share the same attachable-base-pointer (sp or sp->sq in this case).

So, our second invocation for the "remaining component-lists" would become something like:

```
for (Expr* AttachBasePtr: AttachBasePtrs) {
  MapCombinedInfoTy PerAttachBaseCurInfo;
  StructRangeInfoTy PerAttachBasePartialStruct;

// Pass CurInfo to the next two
generateInfoForCaptureForMaps(&PerAttachBaseCurInfo, &PerAttachBasePartialStruct, /*Filter for every component-list that has AttachBaseptr as its attachable base-pointer*/)
emitCombinedEntry(...&PerAttachBaseCurInfo, &PerAttachBasePartialStruct...)
CombinedInfo.append(CurInfo)
}
```

Alexey, do you foresee any issues with this?

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