[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)

[via llvm-branch-commits]

================
@@ -58,21 +67,62 @@ class MapInfoFinalizationPass
            /*corresponding local alloca=*/fir::AllocaOp>
       localBoxAllocas;
 
-  void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
-                               fir::FirOpBuilder &builder,
-                               mlir::Operation *target) {
-    mlir::Location loc = op.getLoc();
-    mlir::Value descriptor = op.getVarPtr();
+  /// getMemberUserList gathers all users of a particular MapInfoOp that are
+  /// other MapInfoOp's and places them into the mapMemberUsers list, which
+  /// records the map that the current argument MapInfoOp "op" is part of
+  /// alongside the placement of "op" in the recorded users members list. The
+  /// intent of the generated list is to find all MapInfoOp's that may be
+  /// considered parents of the passed in "op" and in which it shows up in the
+  /// member list, alongside collecting the placement information of "op" in its
+  /// parents member list.
+  void
+  getMemberUserList(mlir::omp::MapInfoOp op,
+                    llvm::SmallVectorImpl<ParentAndPlacement> &mapMemberUsers) {
----------------
agozillon wrote:

I believe I answered some variation of this question in the previous PR iteration, so just going to copy paste it here: 

"it's intentional for the moment, there should never be more than one mapMemberUsers currently when we reach this pass, as we do not generate MLIR that allows the re-use of MapInfoOp's across operations, so technically the only cases where a MapInfoOp should be used, is a parent map member operation (in which case we add an entry to mapMemberUsers) or when it's tied to an existing MapInfoOp using operation (TargetOp/TargetExitData etc.).

So, perhaps being a bit pre-emptive and adding some initial support for this case, for when we may end up trying to generating tidier MLIR where we re-use map info operations allowing them to have multiple users. As that would be ideal in the future (and I believe you've already written one such test previously).

Previously an assert checking that we had no more than 1 user was above but i think it is covered by the prior assert at line 488 so it felt a little redundant to have two of the same check."

I believe there's another comment at line 271-ish (after updates may be off by a bit) that mentions the above, and I believe I'll be re-adding the assert as requested by @skatrak :-) 

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