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

[via flang-commits]

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

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.

>From caa3615c5b8be795c7f52016089da1bc07759580 Mon Sep 17 00:00:00 2001
From: agozillon <Andrew.Gozillon at amd.com>
Date: Fri, 28 Aug 2026 05:51:26 -0500
Subject: [PATCH] [Flang][OpenMP] Remove close removal code from
 MapInfoFinalization

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, dtypey)

A user should be left to shoot themselves in the foot if that's there goal,
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.
---
 .../Optimizer/OpenMP/MapInfoFinalization.cpp  | 35 -------------------
 .../omp-map-info-finalization-usm.fir         | 24 -------------
 2 files changed, 59 deletions(-)
 delete mode 100644 flang/test/Transforms/omp-map-info-finalization-usm.fir

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