[flang-commits] [flang] [llvm] [Flang][OpenMP] Prevent unrequired implicit maps for private variables on composite directives (PR #217121)

[via flang-commits]

================
@@ -2318,6 +2336,15 @@ static void genBodyOfTargetOp(
     genNestedEvaluations(converter, eval);
   }
 
+  // When materializing arrays inside of the target region for composite
+  // directives with leaf-bound private clauses, we can end up with the
+  // constants populating the shape being bound to constants external to the
+  // target region. Perform a final clone-only repair to resolve these without
+  // introducing new maps after the target operation has already been created.
+  if (!targetNestedPrivateInfo.syms.empty())
----------------
agozillon wrote:

So, when innermostLeafIsTeams = true, this doesn't imply that targetNestedPrivateInfo.syms empty, as we still fill it to prevent extra implicit mappings for them in the implicit mapping segment in the caller to this function and for this particular step.

The innermostLeafIsTeams check above, basically stops us from creating internal clones, as the divergent privatization route it takes (the TODO) does this for us at the moment. However, it is also of note that between the two function calls we handle our leaf lowering including the teams which populates its own internal clones.

The important aspect is that by line 2344 regardless of the route we took (innermostLeafIsTeams false or true) we still have generated internal clones somewhere along our journey, and if targetNestedPrivateInfo.syms isn't empty, then we have clones for them from one of those prior steps, which means there is a chance we have in region operations referencing out of region operations and we need to resolve this (and the map suppression also contributes a little to the issue, as I believe there is a chance it prevents bounds being mapped/copied by the earlier cloneOrMapRegionOutsiders), so we try to do so through cloneOrMapRegionOutsiders, but we restrict it to only do so through cloning and not generation of new maps. As the idea is that any symbol in that list should have been vetted in our prior steps to only depend on constants or otherwise cloneable arguments to type/allocate it.

I did think about this myself before but ran into this issue at the time and that's my understanding of what was going on but it's been a while, and there's perhaps some other way to simplify this but I couldn't think of one at the time :-)

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