[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()) {
+        // 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 (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);
----------------
MattPD wrote:

`memberMap` never contains iterator children, so `removeCloseMapType` never processes them, either in this call or in the branch synthesizing a parent map.

Save the following as `repro.f90`:

```fortran
subroutine close_parent_iterator(x)
  type :: t
    integer :: a(10)
  end type
  type(t) :: x
  !$omp declare mapper(m: t :: v) map(v) map(close, iterator(i=1:2): v%a(i))
  !$omp target data map(mapper(m): x)
  !$omp end target data
end
```

With `flang -fc1 -emit-llvm -fopenmp -fopenmp-version=52 -fopenmp-targets=amdgcn-amd-amdhsa repro.f90 -o -`, Flang warns `OpenMP CLOSE map modifier ignored for structure component`, but the generated mapper still pushes each element with one of the map types 1024 through 1027, each including `OMP_MAP_CLOSE`. For the non-iterator form `map(v) map(close: v%a(1))`, Flang emits the same warning, and the mapper pushes the member without the close bit.

Could `removeCloseMapType` also clear the close bit on the iterator children when the parent map lacks `close`?

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