[llvm-branch-commits] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #111192)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Oct 24 05:55:34 PDT 2024
================
@@ -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
More information about the llvm-branch-commits
mailing list