[flang-commits] [flang] [flang][OpenMP] Enable delayed privatization for `omp parallel` by default (PR #90945)

[Kareem Ergawy via flang-commits]

ergawy wrote:

> Are these issues fixed by #92430?

Unfortunately not. So far I found 3 issues from Fujitsu tests.

#### Cloning `omp.private` without a parent module

This is what causes the sample test Leandro mentioned above. A fix for this would be:
```diff
@@ -1275,7 +1275,11 @@ convertOmpParallel(omp::ParallelOp opInst, llvm::IRBuilderBase &builder,
           // region. The privatizer is processed in-place (see below) before it
           // gets inlined in the parallel region and therefore processing the
           // original op is dangerous.
-          return {privVar, privatizer.clone()};
+
+          mlir::IRRewriter opCloner(&moduleTranslation.getContext());
+          opCloner.setInsertionPoint(privatizer);
+          return {privVar, llvm::cast<mlir::omp::PrivateClauseOp>(
+                               opCloner.clone(*privatizer))};
```

#### Properly mapping common blocks

This resulted in yet another test failing. This would be fixed by something like:
```diff
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 6b391e11beb4..41806f045f82 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1406,12 +1406,23 @@ genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
                       dsp.getAllSymbolsToPrivatize().end());
 
     for (auto [arg, prv] : llvm::zip_equal(allSymbols, region.getArguments())) {
-      converter.bindSymbol(*arg, hlfir::translateToExtendedValue(
-                                     loc, firOpBuilder, hlfir::Entity{prv},
-                                     /*contiguousHint=*/
-                                     evaluate::IsSimplyContiguous(
-                                         *arg, converter.getFoldingContext()))
-                                     .first);
+      mlir::BlockArgument blockArg = prv;
+      auto bind = [&](const semantics::Symbol *sym) {
+        converter.bindSymbol(*sym,
+                             hlfir::translateToExtendedValue(
+                                 loc, firOpBuilder, hlfir::Entity{blockArg},
+                                 /*contiguousHint=*/
+                                 evaluate::IsSimplyContiguous(
+                                     *sym, converter.getFoldingContext()))
+                                 .first);
+      };
+
+      if (const auto *commonDet =
+              arg->detailsIf<semantics::CommonBlockDetails>()) {
+        for (const auto &mem : commonDet->objects())
+          bind(&*mem);
+      } else
+        bind(arg);
     }
```

#### Properly handling common block when some of their items have `equivalence` with other variables

For example, input like this:
```fortran
subroutine private_common
  common /c/ x
  real x, y
  equivalence (x,y)
  !$omp parallel firstprivate(/c/)
    !x = 3.14
    !y = 2.71
  !$omp end parallel
end subroutine
```

Currently produces `omp.private` ops that fail verficiation because the yielded type is not consistent with the symbol type:
```mlir
  omp.private {type = firstprivate} @_QFprivate_commonEx_firstprivate_ptr_f32 : !fir.ptr<f32> alloc {
  ^bb0(%arg0: !fir.ptr<f32>):
    %0 = fir.alloca f32 {bindc_name = "x", pinned, uniq_name = "_QFprivate_commonEx"}
    %1:2 = hlfir.declare %0 {uniq_name = "_QFprivate_commonEx"} : (!fir.ref<f32>) -> (!fir.ref<f32>, !fir.ref<f32>)
    omp.yield(%1#0 : !fir.ref<f32>)
  } copy {
  ^bb0(%arg0: !fir.ptr<f32>, %arg1: !fir.ref<f32>):
    %0 = fir.load %arg0 : !fir.ptr<f32>
    hlfir.assign %0 to %arg1 temporary_lhs : f32, !fir.ref<f32>
    omp.yield(%arg1 : !fir.ref<f32>)
  }
```

Still working on a proper fix for this.

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