[flang-commits] [flang] [Flang][OpenMP] Remove close map type member removal code from MapInfoFinalization (PR #219438)

[via flang-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-openmp

Author: agozillon

<details>
<summary>Changes</summary>

This should no longer be required as we removed the application of close automatically in USM mode in a prior PR. So they bug this was originally put in place for is now addressed without the need for this PR. And whilst it was fine to put in place for a bug the user had no control of (automatic application of close to descriptor members), it's not our place to intervene in scenarios where a user is making explicit errors in their mapping, e.g.

map(to: dtype) map(close, to: dtype%x, dtype%y)

A user should be left to shoot themselves in the foot if that's there goal (or perhaps there's some really wild or single memory system architectures that would allow this), we'd ideally prevent this as a compiler warning or error as opposed to fixing the problem under-the-hood for them. Otherwise we help create unportable code for the user, alongside forcing ourselves to maintain the underlying fix.

---
Full diff: https://github.com/llvm/llvm-project/pull/219438.diff


2 Files Affected:

- (modified) flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp (-35) 
- (removed) flang/test/Transforms/omp-map-info-finalization-usm.fir (-24) 


``````````diff
diff --git a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
index 0fdc2e1278589..7cedce7ca5a13 100644
--- a/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
+++ b/flang/lib/Optimizer/OpenMP/MapInfoFinalization.cpp
@@ -1751,41 +1751,6 @@ class MapInfoFinalizationPass
         }
       });
 
-      func->walk([&](mlir::omp::MapInfoOp op) {
-        // If a record type is not mapped with the `close` modifier while some
-        // of its members are (e.g. descriptor maps), then in USM mode, the
-        // memory for the record will be allocated in unified memory while the
-        // the members might be allocated in device memory. This creates an
-        // inconsistent map for the record type where some of its members are
-        // allocated in different address spaces.
-        //
-        // This fixes this issue by taking a conservative approach and removing
-        // the `close` flag from members if it is not used for mapping the
-        // parent record.
-        if (op.getMembers().empty())
-          return;
-
-        mlir::Type varTy = fir::unwrapRefType(op.getVarPtr().getType());
-        if (!mlir::isa<fir::RecordType>(varTy))
-          return;
-
-        auto mapFlag = op.getMapType();
-        bool hasClose = (mapFlag & mlir::omp::ClauseMapFlags::close) ==
-                        mlir::omp::ClauseMapFlags::close;
-
-        if (hasClose)
-          return;
-
-        for (auto member : op.getMembers()) {
-          if (auto memberOp = llvm::dyn_cast_if_present<mlir::omp::MapInfoOp>(
-                  member.getDefiningOp())) {
-            auto memberMapFlag =
-                memberOp.getMapType() & ~mlir::omp::ClauseMapFlags::close;
-            memberOp.setMapType(memberMapFlag);
-          }
-        }
-      });
-
       // Now that we've expanded all of our boxes into a descriptor and base
       // address map where necessary, we check if the map owner is an
       // enter/exit/target data directive, and if they are we drop the initial
diff --git a/flang/test/Transforms/omp-map-info-finalization-usm.fir b/flang/test/Transforms/omp-map-info-finalization-usm.fir
deleted file mode 100644
index dd1d477323ca1..0000000000000
--- a/flang/test/Transforms/omp-map-info-finalization-usm.fir
+++ /dev/null
@@ -1,24 +0,0 @@
-// RUN: fir-opt --split-input-file --omp-map-info-finalization %s | FileCheck %s
-
-// Test that the 'close' map flag is cleared from member maps if the parent map
-// (derived type) does not have the 'close' flag. This typically happens in
-// Unified Shared Memory (USM) mode where the parent is in USM (no close) but
-// members (like descriptors) might have been initially tagged with close.
-
-module attributes {omp.requires = #omp<clause_requires unified_shared_memory>} {
-  func.func @test_usm_close_flag_cleanup(%arg0: !fir.ref<!fir.type<t{a:!fir.box<!fir.heap<!fir.array<?xf32>>>}>>) {
-    %map = omp.map.info var_ptr(%arg0 : !fir.ref<!fir.type<t{a:!fir.box<!fir.heap<!fir.array<?xf32>>>}>>, !fir.type<t{a:!fir.box<!fir.heap<!fir.array<?xf32>>>}>) map_clauses(to) capture(ByRef) name("parent") -> !fir.ref<!fir.type<t{a:!fir.box<!fir.heap<!fir.array<?xf32>>>}>>
-
-    omp.target kernel_type(generic) map_entries(%map -> %arg1 : !fir.ref<!fir.type<t{a:!fir.box<!fir.heap<!fir.array<?xf32>>>}>>) {
-      // Simulate usage to trigger implicit map addition
-      %1 = hlfir.designate %arg1{"a"} : (!fir.ref<!fir.type<t{a:!fir.box<!fir.heap<!fir.array<?xf32>>>}>>) -> !fir.ref<!fir.box<!fir.heap<!fir.array<?xf32>>>>
-      omp.terminator
-    }
-    return
-  }
-}
-
-// CHECK-LABEL: func.func @test_usm_close_flag_cleanup
-// CHECK: %[[MEMBER:.*]] = omp.map.info {{.*}} map_clauses(always, to) {{.*}} name("parent.a.implicit_map")
-// CHECK: %[[PARENT:.*]] = omp.map.info {{.*}} map_clauses(to) {{.*}} members(%[[MEMBER]], {{.*}}) name("parent") partial_map(true)
-// CHECK-NOT: close

``````````

</details>


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