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

[Tom Eccles 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);
+          }
----------------
tblah wrote:

I wonder if a better fix would be to add support for these types in copyprivate although perhaps the design needs improvement: I would rather we didn't grow yet another re-implementation of allocation/initialization/copy/finalization for all the types flang supports.

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