================
@@ -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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
