[flang-commits] [flang] [flang][OpenMP] incorrect handling for local variable in OpenMP parallel workshare firstprivate(P) (PR #195616)

[Leandro Lupori via flang-commits]

================
@@ -394,12 +435,30 @@ static void parallelizeRegion(Region &sourceRegion, Region &targetRegion,
           allParallelized = false;
         }
       } else if (auto alloca = dyn_cast<fir::AllocaOp>(&op)) {
-        auto hoisted =
-            cast<fir::AllocaOp>(allocaBuilder.clone(*alloca, singleMapping));
-        rootMapping.map(&*alloca, &*hoisted);
-        rootMapping.map(alloca.getResult(), hoisted.getResult());
-        copyPrivate.push_back(hoisted);
-        allParallelized = false;
+        if (alloca.isDynamic()) {
+          // Dynamic allocas (e.g. firstprivate arrays with runtime extent)
+          // cannot use the simple load/store copyprivate copy function
+          // because it only copies a single element for sequence types like
+          // !fir.array<?xi32>. Instead, keep the alloca in the single block
+          // and broadcast only its pointer to all threads.
+          singleBuilder.clone(op, singleMapping);
+          if (isTransitivelyUsedOutside(alloca.getResult(), sr)) {
+            auto alloc =
+                mapReloadedValue(alloca.getResult(), allocaBuilder,
+                                 singleBuilder, parallelBuilder, singleMapping);
+            if (alloc)
+              copyPrivate.push_back(alloc);
+          }
----------------
luporl wrote:

With this change it seems the array isn't firstprivate anymore, as all threads share a single copy of the original array.
Can we guarantee that this won't be an issue due to its use only inside a `workshare` construct?

Another option could be converting the `!fir.array<?xi32>` to a box, since its shape is available.

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