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

[llvmlistbot at llvm.org]

================
@@ -565,6 +565,36 @@ static LogicalResult checkImplementationStatus(Operation &op) {
       result = todo("map/motion clause with iterator modifier");
   };
 
+  // `declare_mapper.info` supports the simple case emitted by Flang's
+  // lowering: a single-dimension iterator whose region yields one bounds-free,
+  // member-free omp.map.info. Anything else still falls back to the generic
+  // (not yet implemented) diagnostic above. Locating the region's unique
+  // `omp.yield` (rather than assuming it terminates the first block) is
+  // required since lowering a scalar subscript (e.g. involving LEN_TRIM) can
+  // introduce control flow inside the iterator.
+  auto checkDeclareMapperMap = [&todo](omp::DeclareMapperInfoOp op,
+                                       LogicalResult &result) {
+    for (Value iterVal : op.getMapIterated()) {
+      auto itersOp = iterVal.getDefiningOp<omp::IteratorOp>();
+      bool supported = itersOp && itersOp.getLoopLowerBounds().size() == 1;
+      if (supported) {
+        omp::YieldOp yieldOp = itersOp.getYieldOp();
+        supported = yieldOp && yieldOp.getResults().size() == 1;
+        if (supported) {
+          auto mapInfoOp =
+              yieldOp.getResults()[0].getDefiningOp<omp::MapInfoOp>();
+          supported = mapInfoOp && !mapInfoOp.getVarPtrPtr() &&
----------------
MattPD wrote:

The `supported` predicate in `checkDeclareMapperMap` accepts every capture kind, although Flang emits only `capture(ByRef)` for iterated map entries.

Save the following as `repro.mlir`:

```mlir
module attributes {omp.target_triples = ["amdgcn-amd-amdhsa"]} {
  omp.declare_mapper @dynamic_copy : !llvm.struct<"s", (i32)> {
  ^bb0(%arg: !llvm.ptr):
    %c0 = llvm.mlir.constant(0 : i64) : i64
    %c1 = llvm.mlir.constant(1 : i64) : i64
    %it = omp.iterator(%iv: i64) = (%c0 to %c0 step %c1) {
      %elem = llvm.getelementptr %arg[%iv] : (!llvm.ptr, i64) -> !llvm.ptr, i32
      %m = omp.map.info var_ptr(%elem : !llvm.ptr, i32) map_clauses(tofrom) capture(ByCopy) name("elem") -> !llvm.ptr
      omp.yield(%m : !llvm.ptr)
    } inclusive -> !omp.iterated<!llvm.ptr>
    omp.declare_mapper.info map_iterated(%it : !omp.iterated<!llvm.ptr>)
  }
  llvm.func @use_dynamic(%addr: !llvm.ptr) {
    %map = omp.map.info var_ptr(%addr : !llvm.ptr, !llvm.struct<"s", (i32)>) map_clauses(tofrom) capture(ByRef) mapper(@dynamic_copy) name("obj") -> !llvm.ptr
    omp.target_data map_entries(%map : !llvm.ptr) {}
    llvm.return
  }
}
```

With `mlir-translate --mlir-to-llvmir repro.mlir`, the generated `.omp_mapper.dynamic_copy` pushes the element address with one of the map types 0 through 3. For the same entry under `map_entries` instead of `map_iterated`, the mapper loads the value and pushes it with `OMP_MAP_LITERAL`, using one of the map types 256 through 259.

Could the `supported` predicate also require `mapInfoOp.getMapCaptureType() == omp::VariableCaptureKind::ByRef`? Translation would then report the not-yet-implemented diagnostic for other capture kinds. The comment on `checkDeclareMapperMap` already says that anything beyond "the simple case emitted by Flang's lowering" falls back to that diagnostic.

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