================
@@ -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)
+      // in this case, should be handled first, to ensure that it gets the
+      // TARGET_PARAM flag.
+      OMPClauseMappableExprCommon::MappableExprComponentListRef Components =
+          std::get<0>(LHS);
+      OMPClauseMappableExprCommon::MappableExprComponentListRef ComponentsR =
+          std::get<0>(RHS);
+      if (VD && VD->getType()->isAnyPointerType() && Components.size() == 1 &&
+          ComponentsR.size() > 1)
+        return true;
----------------
alexey-bataev wrote:

> Is there a case you have in mind? If the number of components is same, then 
> this should not return anything, and we continue to the existing logic and 
> return based on that in line 8855.
> 
> The new return should only kick-in if the current component list has one 
> entry and the other one has more.
> 
> I originally wanted to use the following, which is broader, (and the comment 
> still is in line with this):
> 
> ```c++
> if (VD && VD->getType()->isAnyPointerType() && Components.size() !=
>           ComponentsR.size())
>   return Components.size() < ComponentsR.size();
> ```
> 
> because even when we have `map(p[0][0], p[0])`, the `map(p[0])` should 
> contribute to the kernel argument. `p` is predetermined `firstprivate because 
> `p`is the base-pointer of`map(p[0])`.

I think this one works better.

> 
> The base-pointer for `int **p; ... map(p[0][0])` is `p[0]`, so it has no 
> effect on the data-sharing/mapping of `p`, and hence should not determine the 
> kernel argument.
> 
> But technically we don't need to sort between `p[0][0]` and `p[0][0][0]` 
> since neither of them should ideally contribute to the determination of the 
> data-sharing/mapping clause on `p`.
> 
> Note that sorting between `p[0]` and `p[0][0]` is not needed _yet_, since 
> both `p[0][0]` and `p[0]` use PTR_AND_OBJ mappings that start with `p`.
> 
> Future Plans:
> 
> I am working on a change to support OpenMP 5.0/6.1 compliant 
> pointer-attachment that avoids using the PTR_AND_OBJ mapping for them, and 
> instead uses a new ATTACH map-type bit. With that, the map-chain emitted for 
> `map(present, to: p[0][0]) map(from: p[0])` would look like the following:
> 
> ```c
> // For p[0][0]
> &p[0][0], &p[0][0], sizeof(p[0][0]), TO | PRESENT
> &p[0], &p[0][0], sizeof(ptr), ATTACH
> 
> // For p[0]
> &p[0], &p[0], sizeof(p[0]), FROM | PARAM
> &p, &p[0], ATTACH
> 
> ATTACH:
> * New map-type bit which means don't increase the ref-count, only do an 
> attachment iff either `&p`'s ref-count is 1, or `&p[0]`'s ref-count is 1 
> after finishing all maps, including mappers.
> * Can be combined with ALWAYS for `attach(always)` map-type-modifier to force 
> the attachment even if the ref-counts are more than 1.
> * Can be removed if the user says `attach(never)` (OpenMP 6.1)
> ```
> 
> Once the above is supported, we would need the PARAM bit to apply to the map 
> of `p[0]`, which would likely require sorting between the component-lists, 
> even if neither is of size 1.



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

Reply via email to