[Mlir-commits] [flang] [llvm] [mlir] [flang][mlir][OpenMP] Support iterator modifier in declare mapper map clause (PR #215485)

[llvmlistbot at llvm.org]

================
@@ -610,36 +610,50 @@ void insertChildMapInfoIntoParent(
           mapOperands[std::distance(mapObjects.begin(), parentIter)]
               .getDefiningOp());
 
-      // Once explicit members are attached to a parent map, do not also invoke
-      // a declare mapper on it, otherwise the mapper would remap the same
-      // components leading to duplicate mappings at runtime.
-      if (!indices.second.memberMap.empty() && mapOp.getMapperIdAttr())
-        mapOp.setMapperIdAttr(nullptr);
-
-      // If the explicit parent map itself does not have close mapping, then
-      // close mapping for its explicitly mapped members must be ignored. The
-      // semantic checker emits the corresponding user warning.
-      if ((mapOp.getMapType() & mlir::omp::ClauseMapFlags::close) !=
-          mlir::omp::ClauseMapFlags::close)
-        removeCloseMapType(indices.second);
-
-      // NOTE: To maintain appropriate SSA ordering, we move the parent map
-      // which will now have references to its children after the last
-      // of its members to be generated. This is necessary when a user
-      // has defined a series of parent and children maps where the parent
-      // precedes the children. An alternative, may be to do
-      // delayed generation of map info operations from the clauses and
-      // organize them first before generation. Or to use the
-      // topologicalSort utility which will enforce a stronger SSA
-      // dominance ordering at the cost of efficiency/time.
-      mapOp->moveAfter(indices.second.memberMap.back());
-
-      for (mlir::omp::MapInfoOp memberMap : indices.second.memberMap)
-        mapOp.getMembersMutable().append(memberMap.getResult());
-
-      mapOp.setMembersIndexAttr(firOpBuilder.create2DI64ArrayAttr(
-          indices.second.memberPlacementIndices));
+      // A parent may have only iterator-driven children (e.g. `map(v)` next
+      // to `map(iterator(i=1:1): v%a(i))`); those don't populate memberMap
+      // (they're tracked via mapIterated instead), so there are no static
+      // members to attach to the existing explicit parent map here.
+      if (!indices.second.memberMap.empty()) {
----------------
MattPD wrote:

When the parent has only iterator children, `memberMap` is empty, so the `!indices.second.memberMap.empty()` check also skips `setMapperIdAttr(nullptr)`. `setMapperIdAttr(nullptr)` prevents the parent's mapper from remapping explicitly mapped components.

Save the following as `repro.f90`:

```fortran
subroutine f(x)
  type :: s
    integer :: a(4)
    integer :: b
  end type
  type(s) :: x
  !$omp declare mapper(other: s :: u) map(u%a)
  !$omp declare mapper(m: s :: v) map(mapper(other): v) &
  !$omp& map(iterator(i=1:4): v%a(i))
  !$omp target data map(mapper(m): x)
  !$omp end target data
end subroutine
```

With `flang -fc1 -emit-llvm -fopenmp -fopenmp-version=52 -fopenmp-targets=amdgcn-amd-amdhsa repro.f90 -o -`, the generated `.omp_mapper._QQFfm` first calls `.omp_mapper._QQFfother`, which maps all of `a`. `.omp_mapper._QQFfm` then pushes `v%a(1)` through `v%a(4)` again. With a non-iterator member instead, as in `map(mapper(other): v) map(v%a(1))`, the generated `.omp_mapper._QQFfm` does not call `.omp_mapper._QQFfother`.

Could this code also call `setMapperIdAttr(nullptr)` when the parent has only iterator children, while still skipping the `memberMap.back()` call and the attachment of non-iterator members?

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