================ @@ -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