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

[Sergio Afonso via flang-commits]

================
@@ -164,14 +164,27 @@ mlir::Value mapTemporaryValue(fir::FirOpBuilder &firOpBuilder,
   return loadOp.getResult();
 }
 
-void cloneOrMapRegionOutsiders(
-    fir::FirOpBuilder &firOpBuilder, mlir::omp::TargetOp targetOp) {
+namespace {
+/// Helper function for resolving values used inside a target region but defined
+/// above it. It can resolve through cloning or generating new map info
+/// operations. It will opt for cloning when provably memory effect free,
+/// otherwise it will generate a map, however, only if requested.
+///
+/// \param mapNonClonable - When true, non-clonable outsiders are mapped into
+/// the region; when false they are left untouched.
+/// \param iterateToFixpoint - When true, the set of values-defined-above is
+/// re-queried and reprocessed until emptied. Otherwise a single pass is done.
----------------
skatrak wrote:

If a single pass is done and there are still some operands to the resulting ops inside of the target region defined outside of the region, then this will result in an illegal MLIR module.

My understanding from your comments elsewhere is that this cannot happen, but is only assumed and never checked. If that's the case, we should have an assert rather than an early exit. For example, at the end of the `do-while` loop, check `assert((iterateToFixpoint || valuesDefinedAbove.empty()) && "...")`.

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