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

[llvmlistbot at llvm.org]

================
@@ -2032,13 +2084,138 @@ bool ClauseProcessor::processMap(
       }
     }
 
-    if (iterator)
-      TODO(currentLocation,
-           "Support for iterator modifiers is not implemented yet");
     TodoLocators(currentLocation, objects);
 
-    processMapObjects(stmtCtx, clauseLocation,
-                      std::get<omp::ObjectList>(clause.t), mapTypeBits,
+    if (!iterator) {
+      processMapObjects(stmtCtx, clauseLocation, objects, mapTypeBits,
+                        parentMemberIndices, result.mapVars, *ptrMapObjects,
+                        mapperIdName, /*isMotionModifier=*/false, directive);
+      return;
+    }
+
+    llvm::SmallVector<IteratorRange> iteratorRanges;
+    llvm::SmallPtrSet<const Fortran::semantics::Symbol *, 4> ivSyms;
+    collectIteratorIVs(clause, converter, stmtCtx, iteratorRanges, ivSyms);
+
+    // Objects that reference an iterator induction variable are expanded at
+    // runtime via `omp.iterator`/`map_iterated`; the rest go through the
+    // regular static map-info path below.
+    omp::ObjectList staticObjects;
+    fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+    for (const omp::Object &object : objects) {
+      if (!hasIteratorIVReference(object, ivSyms)) {
+        staticObjects.push_back(object);
+        continue;
+      }
+
+      if (isArraySectionSubscript(object))
+        TODO(currentLocation,
+             "Iterator modifier on an array section in a map clause is not "
+             "implemented yet");
+
+      // The per-iteration `omp.map.info` this produces lives inside the
+      // `omp.iterator` body and cannot be attached as a `members` operand
+      // of some other (parent) MapInfoOp built outside that region, since
+      // only the aggregated `!omp.iterated<T>` handle escapes the region.
+      // We still register the parent with `parentMemberIndices` (with no
+      // child attached) so `insertChildMapInfoIntoParent` synthesizes the
+      // usual partial/"storage" map for the parent object (e.g. the
+      // `declare mapper` association variable itself); the iterator-driven
+      // child map is emitted separately into `result.mapIterated`, mirroring
+      // how `map_vars`/`members` (static) and `map_iterated` (runtime
+      // expanded) already coexist as sibling operands on the owning op.
+      bool hasParentObj = object.sym()->owner().IsDerivedType();
+      mlir::Value baseAddr;
+      if (hasParentObj) {
+        omp::ObjectList objectList = gatherObjectsOf(object, semaCtx);
+        assert(!objectList.empty() &&
+               "could not find parent objects of derived type member");
+        if (isMemberOrParentAllocatableOrPointer(object, semaCtx))
+          TODO(currentLocation,
+               "Iterator modifier on this derived-type member in a map "
+               "clause is not implemented yet");
+
+        omp::Object baseObject = objectList[0];
+        parentMemberIndices.emplace(baseObject, OmpMapParentAndMemberData{});
----------------
chichunchen wrote:

Could we avoid emitting an unconditional parent storage map when the mapper’s only locator expands to an empty set? For example:

```fortran
subroutine f(x)
  type :: t
    integer :: a(10)
  end type
  type(t) :: x

  !$omp declare mapper(m: t :: v) map(iterator(i=3:2): v%a(i))
  !$omp target data map(mapper(m): x)
  !$omp end target data
end
```

The `i` range is empty, so the locator should contribute no mapping. On the current revision, the generated mapper correctly has a zero-trip iterator, but it still pushes a 40-byte parent storage component with map flags 0 before entering that loop. I confirmed this by invoking the generated mapper with host-side runtime stubs.

This appears to come from registering `v` in the parent/member bookkeeping with no static members: parent-map synthesis then creates a standalone storage map regardless of whether the iterator produces any children.

Could we preserve the no-mapping behavior for an empty expansion and add a regression test checking that no component is pushed in this case? This is separate from the folded-reference issue (the locator here still contains `i`, and the iterator’s trip count is already correct).

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