================ @@ -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 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits