[flang-commits] [flang] [flang][MLIR][OpenMP] Extend delayed privatization for arrays and characters (PR #85023)

Kareem Ergawy via flang-commits flang-commits at lists.llvm.org
Mon Apr 29 00:29:41 PDT 2024


https://github.com/ergawy updated https://github.com/llvm/llvm-project/pull/85023

>From 24f62ad49cdd3314933f5225c476e81687dc6a12 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Wed, 13 Mar 2024 00:15:46 -0500
Subject: [PATCH 01/11] [flang][MLIR][OpenMP] Extend delayed privatization for
 arrays

One more step in delayed privatization, this PR extends support for
arrays. In the delayed privatizer, a `fir.box_dims` operation is emitted
to retrieve the array bounds and extents. The result of these
`fir.box_dims` and the privatizer argument, are then used to create a
new `ExtendedValue`.
---
 .../lib/Lower/OpenMP/DataSharingProcessor.cpp | 27 ++++++-
 ...elayed-privatization-allocatable-array.f90 | 67 +++++++++++++++++
 .../OpenMP/delayed-privatization-array.f90    | 75 +++++++++++++++++++
 3 files changed, 167 insertions(+), 2 deletions(-)
 create mode 100644 flang/test/Lower/OpenMP/delayed-privatization-allocatable-array.f90
 create mode 100644 flang/test/Lower/OpenMP/delayed-privatization-array.f90

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index a5c087e4524146..5f2f4101281d12 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -379,8 +379,31 @@ void DataSharingProcessor::doPrivatize(const Fortran::semantics::Symbol *sym) {
           &allocRegion, /*insertPt=*/{}, symType, symLoc);
 
       firOpBuilder.setInsertionPointToEnd(allocEntryBlock);
-      symTable->addSymbol(*sym,
-                          fir::substBase(symExV, allocRegion.getArgument(0)));
+
+      fir::ExtendedValue localExV = symExV.match(
+          [&](const fir::ArrayBoxValue &box) -> fir::ExtendedValue {
+            auto idxTy = firOpBuilder.getIndexType();
+            llvm::SmallVector<mlir::Value> extents;
+            llvm::SmallVector<mlir::Value> lBounds;
+
+            for (unsigned dim = 0; dim < box.getExtents().size(); ++dim) {
+              mlir::Value dimVal =
+                  firOpBuilder.createIntegerConstant(symLoc, idxTy, dim);
+              fir::BoxDimsOp dimInfo = firOpBuilder.create<fir::BoxDimsOp>(
+                  symLoc, idxTy, idxTy, idxTy, allocRegion.getArgument(0),
+                  dimVal);
+              extents.push_back(dimInfo.getExtent());
+              lBounds.push_back(dimInfo.getLowerBound());
+            }
+
+            return fir::ArrayBoxValue(allocRegion.getArgument(0), extents,
+                                      lBounds);
+          },
+          [&](const auto &box) -> fir::ExtendedValue {
+            return fir::substBase(symExV, allocRegion.getArgument(0));
+          });
+
+      symTable->addSymbol(*sym, localExV);
       symTable->pushScope();
       cloneSymbol(sym);
       firOpBuilder.create<mlir::omp::YieldOp>(
diff --git a/flang/test/Lower/OpenMP/delayed-privatization-allocatable-array.f90 b/flang/test/Lower/OpenMP/delayed-privatization-allocatable-array.f90
new file mode 100644
index 00000000000000..47e163014fe868
--- /dev/null
+++ b/flang/test/Lower/OpenMP/delayed-privatization-allocatable-array.f90
@@ -0,0 +1,67 @@
+! Test delayed privatization for allocatable arrays.
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN:   -o - %s 2>&1 | FileCheck %s
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %s 2>&1 |\
+! RUN:   FileCheck %s
+
+subroutine delayed_privatization_private(var1, l1)
+  implicit none
+  integer(8):: l1
+  integer, allocatable, dimension(:) :: var1
+
+!$omp parallel firstprivate(var1)
+  var1(l1 + 1) = 10
+!$omp end parallel
+end subroutine
+
+! CHECK-LABEL: omp.private {type = firstprivate}
+! CHECK-SAME: @[[PRIVATIZER_SYM:.*]] : [[TYPE:!fir.ref<!fir.box<!fir.heap<!fir.array<\?xi32>>>>]] alloc {
+
+! CHECK-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE]]):
+! CHECK-NEXT:   %[[PRIV_ALLOC:.*]] = fir.alloca !fir.box<!fir.heap<!fir.array<{{\?}}xi32>>> {bindc_name = "var1", pinned, uniq_name = "_QFdelayed_privatization_privateEvar1"}
+
+! CHECK-NEXT:   %[[PRIV_ARG_VAL:.*]] = fir.load %[[PRIV_ARG]]
+! CHECK-NEXT:   %[[PRIV_ARG_BOX:.*]] = fir.box_addr %[[PRIV_ARG_VAL]]
+! CHECK-NEXT:   %[[PRIV_ARG_ADDR:.*]] = fir.convert %[[PRIV_ARG_BOX]]
+! CHECK-NEXT:   %[[C0:.*]] = arith.constant 0 : i64
+! CHECK-NEXT:   %[[ALLOC_COND:.*]] = arith.cmpi ne, %[[PRIV_ARG_ADDR]], %[[C0]] : i64
+
+! CHECK-NEXT:   fir.if %[[ALLOC_COND]] {
+! CHECK-NEXT:     %[[PRIV_ARG_VAL:.*]] = fir.load %[[PRIV_ARG]] : [[TYPE]]
+! CHECK-NEXT:     %[[C0:.*]] = arith.constant 0 : index
+! CHECK-NEXT:     %[[DIMS:.*]]:3 = fir.box_dims %[[PRIV_ARG_VAL]], %[[C0]]
+! CHECK-NEXT:     fir.box_addr %[[PRIV_ARG_VAL]]
+! CHECK-NEXT:     %[[C0_2:.*]] = arith.constant 0 : index 
+! CHECK-NEXT:     %[[CMP:.*]] = arith.cmpi sgt, %[[DIMS]]#1, %[[C0_2]] : index
+! CHECK-NEXT:     %[[SELECT:.*]] = arith.select %[[CMP]], %[[DIMS]]#1, %[[C0_2]] : index
+! CHECK-NEXT:     %[[MEM:.*]] = fir.allocmem !fir.array<?xi32>, %[[SELECT]]
+! CHECK-NEXT:     %[[SHAPE_SHIFT:.*]] = fir.shape_shift %[[DIMS]]#0, %[[SELECT]] : (index, index) -> !fir.shapeshift<1>
+! CHECK-NEXT:     %[[EMBOX:.*]] = fir.embox %[[MEM]](%[[SHAPE_SHIFT]])
+! CHECK-NEXT:     fir.store %[[EMBOX]] to %[[PRIV_ALLOC]]
+! CHECK-NEXT:   } else {
+! CHECK-NEXT:     %[[ZEROS:.*]] = fir.zero_bits
+! CHECK-NEXT:     %[[C0_3:.*]] = arith.constant 0 : index
+! CHECK-NEXT:     %[[SHAPE:.*]] = fir.shape %[[C0_3]] : (index) -> !fir.shape<1>
+! CHECK-NEXT:     %[[EMBOX_2:.*]] = fir.embox %[[ZEROS]](%[[SHAPE]])
+! CHECK-NEXT:     fir.store %[[EMBOX_2]] to %[[PRIV_ALLOC]]
+! CHECK-NEXT:   }
+
+! CHECK-NEXT:   hlfir.declare
+! CHECK-NEXT:   omp.yield
+
+! CHECK-NEXT: } copy {
+! CHECK-NEXT: ^bb0(%[[PRIV_ORIG_ARG:.*]]: [[TYPE]], %[[PRIV_PRIV_ARG:.*]]: [[TYPE]]):
+! CHECK-NEXT:  %[[PRIV_BASE_VAL:.*]] = fir.load %[[PRIV_PRIV_ARG]]
+! CHECK-NEXT:  %[[PRIV_BASE_BOX:.*]] = fir.box_addr %[[PRIV_BASE_VAL]]
+! CHECK-NEXT:  %[[PRIV_BASE_ADDR:.*]] = fir.convert %[[PRIV_BASE_BOX]]
+! CHECK-NEXT:  %[[C0:.*]] = arith.constant 0 : i64
+! CHECK-NEXT:  %[[COPY_COND:.*]] = arith.cmpi ne, %[[PRIV_BASE_ADDR]], %[[C0]] : i64
+
+
+! CHECK-NEXT:  fir.if %[[COPY_COND]] {
+! CHECK-NEXT:    %[[PRIV_ORIG_ARG_VAL:.*]] = fir.load %[[PRIV_ORIG_ARG]]
+! CHECK-NEXT:    hlfir.assign %[[PRIV_ORIG_ARG_VAL]] to %[[PRIV_BASE_VAL]] temporary_lhs
+! CHECK-NEXT:   }
+! CHECK-NEXT:   omp.yield
+! CHECK-NEXT: }
diff --git a/flang/test/Lower/OpenMP/delayed-privatization-array.f90 b/flang/test/Lower/OpenMP/delayed-privatization-array.f90
new file mode 100644
index 00000000000000..23b61563781eb1
--- /dev/null
+++ b/flang/test/Lower/OpenMP/delayed-privatization-array.f90
@@ -0,0 +1,75 @@
+! Test delayed privatization for arrays.
+
+! RUN: split-file %s %t
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN:   -o - %t/one_dim_array.f90 2>&1 | FileCheck %s --check-prefix=ONE_DIM
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - \
+! RUN:   %t/one_dim_array.f90 2>&1 | FileCheck %s --check-prefix=ONE_DIM
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN:   -o - %t/two_dim_array.f90 2>&1 | FileCheck %s --check-prefix=TWO_DIM
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %t/two_dim_array.f90 2>&1 |\
+! RUN:   FileCheck %s --check-prefix=TWO_DIM
+
+!--- one_dim_array.f90
+subroutine delayed_privatization_private(var1, l1, u1)
+  implicit none
+  integer(8):: l1, u1
+  integer, dimension(l1:u1) :: var1
+
+!$omp parallel firstprivate(var1)
+  var1(l1 + 1) = 10
+!$omp end parallel
+end subroutine
+
+! ONE_DIM-LABEL: omp.private {type = firstprivate}
+! ONE_DIM-SAME: @[[PRIVATIZER_SYM:.*]] : [[TYPE:!fir.box<!fir.array<\?xi32>>]] alloc {
+
+! ONE_DIM-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE]]):
+
+! ONE_DIM-NEXT:   %[[C0:.*]] = arith.constant 0 : index
+! ONE_DIM-NEXT:   %[[DIMS:.*]]:3 = fir.box_dims %[[PRIV_ARG]], %[[C0]] : ([[TYPE]], index) -> (index, index, index)
+! ONE_DIM-NEXT:   %[[PRIV_ALLOCA:.*]] = fir.alloca !fir.array<{{\?}}xi32>, %[[DIMS]]#1 {bindc_name = "var1", pinned, uniq_name = "_QFdelayed_privatization_privateEvar1"}
+! ONE_DIM-NEXT:   %[[SHAPE_SHIFT:.*]] = fir.shape_shift %[[DIMS]]#0, %[[DIMS]]#1 : (index, index) -> !fir.shapeshift<1>
+! ONE_DIM-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOCA]](%[[SHAPE_SHIFT]]) {uniq_name = "_QFdelayed_privatization_privateEvar1"}
+! ONE_DIM-NEXT:  omp.yield(%[[PRIV_DECL]]#0 : [[TYPE]])
+
+! ONE_DIM-NEXT: } copy {
+! ONE_DIM-NEXT: ^bb0(%[[PRIV_ORIG_ARG:.*]]: [[TYPE]], %[[PRIV_PRIV_ARG:.*]]: [[TYPE]]):
+! ONE_DIM-NEXT:  hlfir.assign %[[PRIV_ORIG_ARG]] to %[[PRIV_PRIV_ARG]] temporary_lhs
+! ONE_DIM-NEXT:   omp.yield(%[[PRIV_PRIV_ARG]] : [[TYPE]])
+! ONE_DIM-NEXT: }
+
+!--- two_dim_array.f90
+subroutine delayed_privatization_private(var1, l1, u1, l2, u2)
+  implicit none
+  integer(8):: l1, u1, l2, u2
+  integer, dimension(l1:u1, l2:u2) :: var1
+
+!$omp parallel firstprivate(var1)
+  var1(l1 + 1, u2) = 10
+!$omp end parallel
+end subroutine
+
+! TWO_DIM-LABEL: omp.private {type = firstprivate}
+! TWO_DIM-SAME: @[[PRIVATIZER_SYM:.*]] : [[TYPE:!fir.box<!fir.array<\?x\?xi32>>]] alloc {
+
+! TWO_DIM-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE]]):
+! TWO_DIM-NEXT:   %[[C0:.*]] = arith.constant 0 : index
+! TWO_DIM-NEXT:   %[[DIMS0:.*]]:3 = fir.box_dims %[[PRIV_ARG]], %[[C0]] : ([[TYPE]], index) -> (index, index, index)
+
+! TWO_DIM-NEXT:   %[[C1:.*]] = arith.constant 1 : index
+! TWO_DIM-NEXT:   %[[DIMS1:.*]]:3 = fir.box_dims %[[PRIV_ARG]], %[[C1]] : ([[TYPE]], index) -> (index, index, index)
+
+! TWO_DIM-NEXT:   %[[PRIV_ALLOCA:.*]] = fir.alloca !fir.array<{{\?}}x{{\?}}xi32>, %[[DIMS0]]#1, %[[DIMS1]]#1 {bindc_name = "var1", pinned, uniq_name = "_QFdelayed_privatization_privateEvar1"}
+! TWO_DIM-NEXT:   %[[SHAPE_SHIFT:.*]] = fir.shape_shift %[[DIMS0]]#0, %[[DIMS0]]#1, %[[DIMS1]]#0, %[[DIMS1]]#1 : (index, index, index, index) -> !fir.shapeshift<2>
+
+! TWO_DIM-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOCA]](%[[SHAPE_SHIFT]]) {uniq_name = "_QFdelayed_privatization_privateEvar1"}
+! TWO_DIM-NEXT:  omp.yield(%[[PRIV_DECL]]#0 : [[TYPE]])
+
+! TWO_DIM-NEXT: } copy {
+! TWO_DIM-NEXT: ^bb0(%[[PRIV_ORIG_ARG:.*]]: [[TYPE]], %[[PRIV_PRIV_ARG:.*]]: [[TYPE]]):
+! TWO_DIM-NEXT:  hlfir.assign %[[PRIV_ORIG_ARG]] to %[[PRIV_PRIV_ARG]] temporary_lhs
+! TWO_DIM-NEXT:   omp.yield(%[[PRIV_PRIV_ARG]] : [[TYPE]])
+! TWO_DIM-NEXT: }

>From 943c7e21d7b2b8a126a0de02e6cb3aac1ce3d680 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Wed, 17 Apr 2024 23:32:28 -0500
Subject: [PATCH 02/11] Use `genLboundsAndExtentsFromBox`.

---
 .../flang/Optimizer/Builder/HLFIRTools.h        |  4 ++++
 flang/lib/Lower/OpenMP/DataSharingProcessor.cpp | 17 +++++------------
 flang/lib/Optimizer/Builder/HLFIRTools.cpp      | 17 ++++++++---------
 3 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/flang/include/flang/Optimizer/Builder/HLFIRTools.h b/flang/include/flang/Optimizer/Builder/HLFIRTools.h
index 035035601e2f25..a93c263d4aac01 100644
--- a/flang/include/flang/Optimizer/Builder/HLFIRTools.h
+++ b/flang/include/flang/Optimizer/Builder/HLFIRTools.h
@@ -462,6 +462,10 @@ genTypeAndKindConvert(mlir::Location loc, fir::FirOpBuilder &builder,
                       hlfir::Entity source, mlir::Type toType,
                       bool preserveLowerBounds);
 
+void genLboundsAndExtentsFromBox(mlir::Location loc, fir::FirOpBuilder &builder,
+                                 mlir::Value boxEntity, int rank,
+                                 llvm::SmallVectorImpl<mlir::Value> &lbounds,
+                                 llvm::SmallVectorImpl<mlir::Value> *extents);
 } // namespace hlfir
 
 #endif // FORTRAN_OPTIMIZER_BUILDER_HLFIRTOOLS_H
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index b8a82e2e06321f..09a9e0dbce9a55 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -15,6 +15,7 @@
 #include "Utils.h"
 #include "flang/Lower/PFTBuilder.h"
 #include "flang/Lower/SymbolMap.h"
+#include "flang/Optimizer/Builder/HLFIRTools.h"
 #include "flang/Optimizer/Builder/Todo.h"
 #include "flang/Semantics/tools.h"
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
@@ -393,25 +394,17 @@ void DataSharingProcessor::doPrivatize(
 
       fir::ExtendedValue localExV = symExV.match(
           [&](const fir::ArrayBoxValue &box) -> fir::ExtendedValue {
-            auto idxTy = firOpBuilder.getIndexType();
             llvm::SmallVector<mlir::Value> extents;
             llvm::SmallVector<mlir::Value> lBounds;
-
-            for (unsigned dim = 0; dim < box.getExtents().size(); ++dim) {
-              mlir::Value dimVal =
-                  firOpBuilder.createIntegerConstant(symLoc, idxTy, dim);
-              fir::BoxDimsOp dimInfo = firOpBuilder.create<fir::BoxDimsOp>(
-                  symLoc, idxTy, idxTy, idxTy, allocRegion.getArgument(0),
-                  dimVal);
-              extents.push_back(dimInfo.getExtent());
-              lBounds.push_back(dimInfo.getLowerBound());
-            }
+            hlfir::genLboundsAndExtentsFromBox(symLoc, firOpBuilder,
+                                               allocRegion.getArgument(0),
+                                               box.rank(), lBounds, &extents);
 
             return fir::ArrayBoxValue(allocRegion.getArgument(0), extents,
                                       lBounds);
           },
           [&](const auto &box) -> fir::ExtendedValue {
-            return fir::substBase(symExV, allocRegion.getArgument(0));
+            return fir::substBase(box, allocRegion.getArgument(0));
           });
 
       symTable->addSymbol(*sym, localExV);
diff --git a/flang/lib/Optimizer/Builder/HLFIRTools.cpp b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
index db638ceb40700b..60b251df2d7cd2 100644
--- a/flang/lib/Optimizer/Builder/HLFIRTools.cpp
+++ b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
@@ -94,14 +94,12 @@ getExplicitLbounds(fir::FortranVariableOpInterface var) {
   return {};
 }
 
-static void
-genLboundsAndExtentsFromBox(mlir::Location loc, fir::FirOpBuilder &builder,
-                            hlfir::Entity boxEntity,
-                            llvm::SmallVectorImpl<mlir::Value> &lbounds,
-                            llvm::SmallVectorImpl<mlir::Value> *extents) {
+void hlfir::genLboundsAndExtentsFromBox(
+    mlir::Location loc, fir::FirOpBuilder &builder, mlir::Value boxEntity,
+    int rank, llvm::SmallVectorImpl<mlir::Value> &lbounds,
+    llvm::SmallVectorImpl<mlir::Value> *extents) {
   assert(boxEntity.getType().isa<fir::BaseBoxType>() && "must be a box");
   mlir::Type idxTy = builder.getIndexType();
-  const int rank = boxEntity.getRank();
   for (int i = 0; i < rank; ++i) {
     mlir::Value dim = builder.createIntegerConstant(loc, idxTy, i);
     auto dimInfo = builder.create<fir::BoxDimsOp>(loc, idxTy, idxTy, idxTy,
@@ -125,7 +123,8 @@ getNonDefaultLowerBounds(mlir::Location loc, fir::FirOpBuilder &builder,
   if (entity.isMutableBox())
     entity = hlfir::derefPointersAndAllocatables(loc, builder, entity);
   llvm::SmallVector<mlir::Value> lowerBounds;
-  genLboundsAndExtentsFromBox(loc, builder, entity, lowerBounds,
+  genLboundsAndExtentsFromBox(loc, builder, entity, entity.getRank(),
+                              lowerBounds,
                               /*extents=*/nullptr);
   return lowerBounds;
 }
@@ -887,8 +886,8 @@ translateVariableToExtendedValue(mlir::Location loc, fir::FirOpBuilder &builder,
       !variable.getIfVariableInterface()) {
     // This special case avoids generating two sets of identical
     // fir.box_dim to get both the lower bounds and extents.
-    genLboundsAndExtentsFromBox(loc, builder, variable, nonDefaultLbounds,
-                                &extents);
+    genLboundsAndExtentsFromBox(loc, builder, variable, variable.getRank(),
+                                nonDefaultLbounds, &extents);
   } else {
     extents = getVariableExtents(loc, builder, variable);
     nonDefaultLbounds = getNonDefaultLowerBounds(loc, builder, variable);

>From 81f422d18df2c5f2df4e8b61fe047f59ddd6f7da Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Wed, 17 Apr 2024 23:44:11 -0500
Subject: [PATCH 03/11] Add explicit todos for unsupported extended value
 types.

---
 .../lib/Lower/OpenMP/DataSharingProcessor.cpp | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 09a9e0dbce9a55..11d68c68a2fd6e 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -403,6 +403,31 @@ void DataSharingProcessor::doPrivatize(
             return fir::ArrayBoxValue(allocRegion.getArgument(0), extents,
                                       lBounds);
           },
+          [&](const fir::CharBoxValue &box) -> fir::ExtendedValue {
+            TODO(symLoc,
+                 "Delayed privatization is not supported for CharBoxValue");
+            return {};
+          },
+          [&](const fir::CharArrayBoxValue &box) -> fir::ExtendedValue {
+            TODO(
+                symLoc,
+                "Delayed privatization is not supported for CharArrayBoxValue");
+            return {};
+          },
+          [&](const fir::ProcBoxValue &box) -> fir::ExtendedValue {
+            TODO(symLoc,
+                 "Delayed privatization is not supported for ProcBoxValue");
+            return {};
+          },
+          [&](const fir::BoxValue &box) -> fir::ExtendedValue {
+            TODO(symLoc, "Delayed privatization is not supported for BoxValue");
+            return {};
+          },
+          [&](const fir::PolymorphicValue &box) -> fir::ExtendedValue {
+            TODO(symLoc,
+                 "Delayed privatization is not supported for PolymorphicValue");
+            return {};
+          },
           [&](const auto &box) -> fir::ExtendedValue {
             return fir::substBase(box, allocRegion.getArgument(0));
           });

>From 738962b7278a0ffdb76a92d992176a8c29157ce8 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Wed, 24 Apr 2024 02:14:43 -0500
Subject: [PATCH 04/11] Use `translateToExtendedValue`.

---
 .../flang/Optimizer/Builder/HLFIRTools.h      |  6 +----
 .../lib/Lower/OpenMP/DataSharingProcessor.cpp | 12 +++------
 flang/lib/Lower/OpenMP/OpenMP.cpp             |  6 +++--
 flang/lib/Optimizer/Builder/HLFIRTools.cpp    | 25 +++++++++++--------
 .../OpenMP/delayed-privatization-array.f90    |  6 ++---
 5 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/flang/include/flang/Optimizer/Builder/HLFIRTools.h b/flang/include/flang/Optimizer/Builder/HLFIRTools.h
index a93c263d4aac01..a472b645781dcf 100644
--- a/flang/include/flang/Optimizer/Builder/HLFIRTools.h
+++ b/flang/include/flang/Optimizer/Builder/HLFIRTools.h
@@ -223,7 +223,7 @@ class EntityWithAttributes : public Entity {
 using CleanupFunction = std::function<void()>;
 std::pair<fir::ExtendedValue, std::optional<CleanupFunction>>
 translateToExtendedValue(mlir::Location loc, fir::FirOpBuilder &builder,
-                         Entity entity);
+                         Entity entity, bool contiguousHint = false);
 
 /// Function to translate FortranVariableOpInterface to fir::ExtendedValue.
 /// It may generates IR to unbox fir.boxchar, but has otherwise no side effects
@@ -462,10 +462,6 @@ genTypeAndKindConvert(mlir::Location loc, fir::FirOpBuilder &builder,
                       hlfir::Entity source, mlir::Type toType,
                       bool preserveLowerBounds);
 
-void genLboundsAndExtentsFromBox(mlir::Location loc, fir::FirOpBuilder &builder,
-                                 mlir::Value boxEntity, int rank,
-                                 llvm::SmallVectorImpl<mlir::Value> &lbounds,
-                                 llvm::SmallVectorImpl<mlir::Value> *extents);
 } // namespace hlfir
 
 #endif // FORTRAN_OPTIMIZER_BUILDER_HLFIRTOOLS_H
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 2e7eae2d7ba749..5268b7bdf358c5 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -392,14 +392,10 @@ void DataSharingProcessor::doPrivatize(
 
       fir::ExtendedValue localExV = symExV.match(
           [&](const fir::ArrayBoxValue &box) -> fir::ExtendedValue {
-            llvm::SmallVector<mlir::Value> extents;
-            llvm::SmallVector<mlir::Value> lBounds;
-            hlfir::genLboundsAndExtentsFromBox(symLoc, firOpBuilder,
-                                               allocRegion.getArgument(0),
-                                               box.rank(), lBounds, &extents);
-
-            return fir::ArrayBoxValue(allocRegion.getArgument(0), extents,
-                                      lBounds);
+            return hlfir::translateToExtendedValue(
+                       symLoc, firOpBuilder,
+                       hlfir::Entity{allocRegion.getArgument(0)}, true)
+                .first;
           },
           [&](const fir::CharBoxValue &box) -> fir::ExtendedValue {
             TODO(symLoc,
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index e932f7c284bca8..bfde08eabf287c 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1459,13 +1459,15 @@ genParallelOp(Fortran::lower::AbstractConverter &converter,
         reductionSyms;
     allSymbols.append(privateSyms);
     for (auto [arg, prv] : llvm::zip_equal(allSymbols, region.getArguments())) {
-      converter.bindSymbol(*arg, prv);
+      converter.bindSymbol(*arg, hlfir::translateToExtendedValue(
+                                     loc, firOpBuilder, hlfir::Entity{prv},
+                                     /*contiguousHint=*/true)
+                                     .first);
     }
 
     return allSymbols;
   };
 
-  // TODO Merge with the reduction CB.
   genInfo.setGenRegionEntryCb(genRegionEntryCB).setDataSharingProcessor(&dsp);
   return genOpWithBody<mlir::omp::ParallelOp>(genInfo, clauseOps);
 }
diff --git a/flang/lib/Optimizer/Builder/HLFIRTools.cpp b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
index 60b251df2d7cd2..740244c9d75c75 100644
--- a/flang/lib/Optimizer/Builder/HLFIRTools.cpp
+++ b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
@@ -94,10 +94,11 @@ getExplicitLbounds(fir::FortranVariableOpInterface var) {
   return {};
 }
 
-void hlfir::genLboundsAndExtentsFromBox(
-    mlir::Location loc, fir::FirOpBuilder &builder, mlir::Value boxEntity,
-    int rank, llvm::SmallVectorImpl<mlir::Value> &lbounds,
-    llvm::SmallVectorImpl<mlir::Value> *extents) {
+static void
+genLboundsAndExtentsFromBox(mlir::Location loc, fir::FirOpBuilder &builder,
+                            hlfir::Entity boxEntity, int rank,
+                            llvm::SmallVectorImpl<mlir::Value> &lbounds,
+                            llvm::SmallVectorImpl<mlir::Value> *extents) {
   assert(boxEntity.getType().isa<fir::BaseBoxType>() && "must be a box");
   mlir::Type idxTy = builder.getIndexType();
   for (int i = 0; i < rank; ++i) {
@@ -845,10 +846,9 @@ hlfir::LoopNest hlfir::genLoopNest(mlir::Location loc,
   return loopNest;
 }
 
-static fir::ExtendedValue
-translateVariableToExtendedValue(mlir::Location loc, fir::FirOpBuilder &builder,
-                                 hlfir::Entity variable,
-                                 bool forceHlfirBase = false) {
+static fir::ExtendedValue translateVariableToExtendedValue(
+    mlir::Location loc, fir::FirOpBuilder &builder, hlfir::Entity variable,
+    bool forceHlfirBase = false, bool contiguousHint = false) {
   assert(variable.isVariable() && "must be a variable");
   /// When going towards FIR, use the original base value to avoid
   /// introducing descriptors at runtime when they are not required.
@@ -859,7 +859,9 @@ translateVariableToExtendedValue(mlir::Location loc, fir::FirOpBuilder &builder,
                                 fir::MutableProperties{});
 
   if (base.getType().isa<fir::BaseBoxType>()) {
-    if (!variable.isSimplyContiguous() || variable.isPolymorphic() ||
+
+    bool contiguous = variable.isSimplyContiguous() || contiguousHint;
+    if (!contiguous || variable.isPolymorphic() ||
         variable.isDerivedWithLengthParameters() || variable.isOptional()) {
       llvm::SmallVector<mlir::Value> nonDefaultLbounds =
           getNonDefaultLowerBounds(loc, builder, variable);
@@ -908,9 +910,10 @@ hlfir::translateToExtendedValue(mlir::Location loc, fir::FirOpBuilder &builder,
 
 std::pair<fir::ExtendedValue, std::optional<hlfir::CleanupFunction>>
 hlfir::translateToExtendedValue(mlir::Location loc, fir::FirOpBuilder &builder,
-                                hlfir::Entity entity) {
+                                hlfir::Entity entity, bool contiguousHint) {
   if (entity.isVariable())
-    return {translateVariableToExtendedValue(loc, builder, entity),
+    return {translateVariableToExtendedValue(loc, builder, entity, false,
+                                             contiguousHint),
             std::nullopt};
 
   if (entity.isProcedure()) {
diff --git a/flang/test/Lower/OpenMP/delayed-privatization-array.f90 b/flang/test/Lower/OpenMP/delayed-privatization-array.f90
index 23b61563781eb1..cb79a5166dc046 100644
--- a/flang/test/Lower/OpenMP/delayed-privatization-array.f90
+++ b/flang/test/Lower/OpenMP/delayed-privatization-array.f90
@@ -28,9 +28,9 @@ subroutine delayed_privatization_private(var1, l1, u1)
 
 ! ONE_DIM-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE]]):
 
-! ONE_DIM-NEXT:   %[[C0:.*]] = arith.constant 0 : index
+! ONE_DIM:   %[[C0:.*]] = arith.constant 0 : index
 ! ONE_DIM-NEXT:   %[[DIMS:.*]]:3 = fir.box_dims %[[PRIV_ARG]], %[[C0]] : ([[TYPE]], index) -> (index, index, index)
-! ONE_DIM-NEXT:   %[[PRIV_ALLOCA:.*]] = fir.alloca !fir.array<{{\?}}xi32>, %[[DIMS]]#1 {bindc_name = "var1", pinned, uniq_name = "_QFdelayed_privatization_privateEvar1"}
+! ONE_DIM:   %[[PRIV_ALLOCA:.*]] = fir.alloca !fir.array<{{\?}}xi32>
 ! ONE_DIM-NEXT:   %[[SHAPE_SHIFT:.*]] = fir.shape_shift %[[DIMS]]#0, %[[DIMS]]#1 : (index, index) -> !fir.shapeshift<1>
 ! ONE_DIM-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOCA]](%[[SHAPE_SHIFT]]) {uniq_name = "_QFdelayed_privatization_privateEvar1"}
 ! ONE_DIM-NEXT:  omp.yield(%[[PRIV_DECL]]#0 : [[TYPE]])
@@ -56,7 +56,7 @@ subroutine delayed_privatization_private(var1, l1, u1, l2, u2)
 ! TWO_DIM-SAME: @[[PRIVATIZER_SYM:.*]] : [[TYPE:!fir.box<!fir.array<\?x\?xi32>>]] alloc {
 
 ! TWO_DIM-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE]]):
-! TWO_DIM-NEXT:   %[[C0:.*]] = arith.constant 0 : index
+! TWO_DIM:        %[[C0:.*]] = arith.constant 0 : index
 ! TWO_DIM-NEXT:   %[[DIMS0:.*]]:3 = fir.box_dims %[[PRIV_ARG]], %[[C0]] : ([[TYPE]], index) -> (index, index, index)
 
 ! TWO_DIM-NEXT:   %[[C1:.*]] = arith.constant 1 : index

>From 080a28588ea4efec0ecb857b9da459652b59294e Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Wed, 24 Apr 2024 22:59:10 -0500
Subject: [PATCH 05/11] clean-ups

---
 flang/lib/Lower/OpenMP/OpenMP.cpp          |  5 +++--
 flang/lib/Optimizer/Builder/HLFIRTools.cpp | 10 +++++-----
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 48c3aa85c25f35..847763dc4db11c 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1427,9 +1427,10 @@ genParallelOp(Fortran::lower::AbstractConverter &converter,
         reductionSyms;
     allSymbols.append(privateSyms);
     for (auto [arg, prv] : llvm::zip_equal(allSymbols, region.getArguments())) {
+      hlfir::Entity regionArgEntity{prv};
       converter.bindSymbol(*arg, hlfir::translateToExtendedValue(
-                                     loc, firOpBuilder, hlfir::Entity{prv},
-                                     /*contiguousHint=*/true)
+                                     loc, firOpBuilder, regionArgEntity,
+                                     regionArgEntity.isArray())
                                      .first);
     }
 
diff --git a/flang/lib/Optimizer/Builder/HLFIRTools.cpp b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
index 740244c9d75c75..db18750c0c63cf 100644
--- a/flang/lib/Optimizer/Builder/HLFIRTools.cpp
+++ b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
@@ -96,11 +96,12 @@ getExplicitLbounds(fir::FortranVariableOpInterface var) {
 
 static void
 genLboundsAndExtentsFromBox(mlir::Location loc, fir::FirOpBuilder &builder,
-                            hlfir::Entity boxEntity, int rank,
+                            hlfir::Entity boxEntity,
                             llvm::SmallVectorImpl<mlir::Value> &lbounds,
                             llvm::SmallVectorImpl<mlir::Value> *extents) {
   assert(boxEntity.getType().isa<fir::BaseBoxType>() && "must be a box");
   mlir::Type idxTy = builder.getIndexType();
+  const int rank = boxEntity.getRank();
   for (int i = 0; i < rank; ++i) {
     mlir::Value dim = builder.createIntegerConstant(loc, idxTy, i);
     auto dimInfo = builder.create<fir::BoxDimsOp>(loc, idxTy, idxTy, idxTy,
@@ -124,8 +125,7 @@ getNonDefaultLowerBounds(mlir::Location loc, fir::FirOpBuilder &builder,
   if (entity.isMutableBox())
     entity = hlfir::derefPointersAndAllocatables(loc, builder, entity);
   llvm::SmallVector<mlir::Value> lowerBounds;
-  genLboundsAndExtentsFromBox(loc, builder, entity, entity.getRank(),
-                              lowerBounds,
+  genLboundsAndExtentsFromBox(loc, builder, entity, lowerBounds,
                               /*extents=*/nullptr);
   return lowerBounds;
 }
@@ -888,8 +888,8 @@ static fir::ExtendedValue translateVariableToExtendedValue(
       !variable.getIfVariableInterface()) {
     // This special case avoids generating two sets of identical
     // fir.box_dim to get both the lower bounds and extents.
-    genLboundsAndExtentsFromBox(loc, builder, variable, variable.getRank(),
-                                nonDefaultLbounds, &extents);
+    genLboundsAndExtentsFromBox(loc, builder, variable, nonDefaultLbounds,
+                                &extents);
   } else {
     extents = getVariableExtents(loc, builder, variable);
     nonDefaultLbounds = getNonDefaultLowerBounds(loc, builder, variable);

>From 0d13dbb0af93ea2a3693be4179271457f0ecd5ea Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Wed, 24 Apr 2024 23:56:22 -0500
Subject: [PATCH 06/11] Extend uses for

---
 flang/lib/Lower/OpenMP/DataSharingProcessor.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index cfcba26eb19996..e269cbca87c73d 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -430,7 +430,10 @@ void DataSharingProcessor::doPrivatize(
             return {};
           },
           [&](const auto &box) -> fir::ExtendedValue {
-            return fir::substBase(box, allocRegion.getArgument(0));
+            return hlfir::translateToExtendedValue(
+                       symLoc, firOpBuilder,
+                       hlfir::Entity{allocRegion.getArgument(0)}, true)
+                .first;
           });
 
       symTable->addSymbol(*sym, localExV);

>From 05a68a6c1d739172028c9e883584f21b5a6d48cc Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Thu, 25 Apr 2024 03:12:55 -0500
Subject: [PATCH 07/11] Add one more test.

---
 .../OpenMP/delayed-privatization-array.f90    | 39 +++++++++++++++----
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/flang/test/Lower/OpenMP/delayed-privatization-array.f90 b/flang/test/Lower/OpenMP/delayed-privatization-array.f90
index cb79a5166dc046..1d291b2ac0febd 100644
--- a/flang/test/Lower/OpenMP/delayed-privatization-array.f90
+++ b/flang/test/Lower/OpenMP/delayed-privatization-array.f90
@@ -9,11 +9,16 @@
 
 ! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
 ! RUN:   -o - %t/two_dim_array.f90 2>&1 | FileCheck %s --check-prefix=TWO_DIM
-! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %t/two_dim_array.f90 2>&1 |\
-! RUN:   FileCheck %s --check-prefix=TWO_DIM
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - \
+! RUN:   %t/two_dim_array.f90 2>&1 | FileCheck %s --check-prefix=TWO_DIM
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN:   -o - %t/one_dim_array_default_lb.f90 2>&1 | FileCheck %s --check-prefix=ONE_DIM_DEFAULT_LB
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - \
+! RUN:   %t/one_dim_array_default_lb.f90 2>&1 | FileCheck %s --check-prefix=ONE_DIM_DEFAULT_LB
 
 !--- one_dim_array.f90
-subroutine delayed_privatization_private(var1, l1, u1)
+subroutine delayed_privatization_private_1d(var1, l1, u1)
   implicit none
   integer(8):: l1, u1
   integer, dimension(l1:u1) :: var1
@@ -32,7 +37,7 @@ subroutine delayed_privatization_private(var1, l1, u1)
 ! ONE_DIM-NEXT:   %[[DIMS:.*]]:3 = fir.box_dims %[[PRIV_ARG]], %[[C0]] : ([[TYPE]], index) -> (index, index, index)
 ! ONE_DIM:   %[[PRIV_ALLOCA:.*]] = fir.alloca !fir.array<{{\?}}xi32>
 ! ONE_DIM-NEXT:   %[[SHAPE_SHIFT:.*]] = fir.shape_shift %[[DIMS]]#0, %[[DIMS]]#1 : (index, index) -> !fir.shapeshift<1>
-! ONE_DIM-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOCA]](%[[SHAPE_SHIFT]]) {uniq_name = "_QFdelayed_privatization_privateEvar1"}
+! ONE_DIM-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOCA]](%[[SHAPE_SHIFT]]) {uniq_name = "_QFdelayed_privatization_private_1dEvar1"}
 ! ONE_DIM-NEXT:  omp.yield(%[[PRIV_DECL]]#0 : [[TYPE]])
 
 ! ONE_DIM-NEXT: } copy {
@@ -42,7 +47,7 @@ subroutine delayed_privatization_private(var1, l1, u1)
 ! ONE_DIM-NEXT: }
 
 !--- two_dim_array.f90
-subroutine delayed_privatization_private(var1, l1, u1, l2, u2)
+subroutine delayed_privatization_private_2d(var1, l1, u1, l2, u2)
   implicit none
   integer(8):: l1, u1, l2, u2
   integer, dimension(l1:u1, l2:u2) :: var1
@@ -62,10 +67,10 @@ subroutine delayed_privatization_private(var1, l1, u1, l2, u2)
 ! TWO_DIM-NEXT:   %[[C1:.*]] = arith.constant 1 : index
 ! TWO_DIM-NEXT:   %[[DIMS1:.*]]:3 = fir.box_dims %[[PRIV_ARG]], %[[C1]] : ([[TYPE]], index) -> (index, index, index)
 
-! TWO_DIM-NEXT:   %[[PRIV_ALLOCA:.*]] = fir.alloca !fir.array<{{\?}}x{{\?}}xi32>, %[[DIMS0]]#1, %[[DIMS1]]#1 {bindc_name = "var1", pinned, uniq_name = "_QFdelayed_privatization_privateEvar1"}
+! TWO_DIM-NEXT:   %[[PRIV_ALLOCA:.*]] = fir.alloca !fir.array<{{\?}}x{{\?}}xi32>, %[[DIMS0]]#1, %[[DIMS1]]#1 {bindc_name = "var1", pinned, uniq_name = "_QFdelayed_privatization_private_2dEvar1"}
 ! TWO_DIM-NEXT:   %[[SHAPE_SHIFT:.*]] = fir.shape_shift %[[DIMS0]]#0, %[[DIMS0]]#1, %[[DIMS1]]#0, %[[DIMS1]]#1 : (index, index, index, index) -> !fir.shapeshift<2>
 
-! TWO_DIM-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOCA]](%[[SHAPE_SHIFT]]) {uniq_name = "_QFdelayed_privatization_privateEvar1"}
+! TWO_DIM-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOCA]](%[[SHAPE_SHIFT]]) {uniq_name = "_QFdelayed_privatization_private_2dEvar1"}
 ! TWO_DIM-NEXT:  omp.yield(%[[PRIV_DECL]]#0 : [[TYPE]])
 
 ! TWO_DIM-NEXT: } copy {
@@ -73,3 +78,23 @@ subroutine delayed_privatization_private(var1, l1, u1, l2, u2)
 ! TWO_DIM-NEXT:  hlfir.assign %[[PRIV_ORIG_ARG]] to %[[PRIV_PRIV_ARG]] temporary_lhs
 ! TWO_DIM-NEXT:   omp.yield(%[[PRIV_PRIV_ARG]] : [[TYPE]])
 ! TWO_DIM-NEXT: }
+
+!--- one_dim_array_default_lb.f90
+program main
+  implicit none
+  integer, dimension(10) :: var1
+
+!$omp parallel private(var1)
+  var1(1) = 10
+!$omp end parallel
+end program
+
+! ONE_DIM_DEFAULT_LB-LABEL: omp.private {type = private}
+! ONE_DIM_DEFAULT_LB-SAME: @[[PRIVATIZER_SYM:.*]] : [[TYPE:!fir.ref<!fir.array<10xi32>>]] alloc {
+
+! ONE_DIM_DEFAULT_LB-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE]]):
+
+! ONE_DIM_DEFAULT_LB:   %[[C10:.*]] = arith.constant 10 : index
+! ONE_DIM_DEFAULT_LB:   %[[PRIV_ALLOCA:.*]] = fir.alloca !fir.array<10xi32>
+! ONE_DIM_DEFAULT_LB:   %[[SHAPE:.*]] = fir.shape %[[C10]] : (index) -> !fir.shape<1>
+! ONE_DIM_DEFAULT_LB:   hlfir.declare %[[PRIV_ALLOCA]](%[[SHAPE]])

>From 1692791665b00f9229aecef63e6ca32682c90147 Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Thu, 25 Apr 2024 22:45:02 -0500
Subject: [PATCH 08/11] More clean-up

---
 .../lib/Lower/OpenMP/DataSharingProcessor.cpp | 47 ++++---------------
 flang/lib/Lower/OpenMP/OpenMP.cpp             |  8 ++--
 2 files changed, 14 insertions(+), 41 deletions(-)

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index e269cbca87c73d..66179012e24c43 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -19,6 +19,7 @@
 #include "flang/Optimizer/Builder/Todo.h"
 #include "flang/Semantics/tools.h"
 #include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include <variant>
 
 namespace Fortran {
 namespace lower {
@@ -397,44 +398,14 @@ void DataSharingProcessor::doPrivatize(
 
       firOpBuilder.setInsertionPointToEnd(allocEntryBlock);
 
-      fir::ExtendedValue localExV = symExV.match(
-          [&](const fir::ArrayBoxValue &box) -> fir::ExtendedValue {
-            return hlfir::translateToExtendedValue(
-                       symLoc, firOpBuilder,
-                       hlfir::Entity{allocRegion.getArgument(0)}, true)
-                .first;
-          },
-          [&](const fir::CharBoxValue &box) -> fir::ExtendedValue {
-            TODO(symLoc,
-                 "Delayed privatization is not supported for CharBoxValue");
-            return {};
-          },
-          [&](const fir::CharArrayBoxValue &box) -> fir::ExtendedValue {
-            TODO(
-                symLoc,
-                "Delayed privatization is not supported for CharArrayBoxValue");
-            return {};
-          },
-          [&](const fir::ProcBoxValue &box) -> fir::ExtendedValue {
-            TODO(symLoc,
-                 "Delayed privatization is not supported for ProcBoxValue");
-            return {};
-          },
-          [&](const fir::BoxValue &box) -> fir::ExtendedValue {
-            TODO(symLoc, "Delayed privatization is not supported for BoxValue");
-            return {};
-          },
-          [&](const fir::PolymorphicValue &box) -> fir::ExtendedValue {
-            TODO(symLoc,
-                 "Delayed privatization is not supported for PolymorphicValue");
-            return {};
-          },
-          [&](const auto &box) -> fir::ExtendedValue {
-            return hlfir::translateToExtendedValue(
-                       symLoc, firOpBuilder,
-                       hlfir::Entity{allocRegion.getArgument(0)}, true)
-                .first;
-          });
+      // TODO Delayed privatization has not been tested yet for: CharBoxValue,
+      // CharArrayBoxValue, BoxValue, or PolymorphicValue.
+      fir::ExtendedValue localExV =
+          hlfir::translateToExtendedValue(
+              symLoc, firOpBuilder, hlfir::Entity{allocRegion.getArgument(0)},
+              /*contiguousHint=*/
+              std::holds_alternative<fir::ArrayBoxValue>(symExV.matchee()))
+              .first;
 
       symTable->addSymbol(*sym, localExV);
       symTable->pushScope();
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 847763dc4db11c..8579e0047e8c40 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1427,10 +1427,12 @@ genParallelOp(Fortran::lower::AbstractConverter &converter,
         reductionSyms;
     allSymbols.append(privateSyms);
     for (auto [arg, prv] : llvm::zip_equal(allSymbols, region.getArguments())) {
-      hlfir::Entity regionArgEntity{prv};
+      fir::ExtendedValue hostExV = converter.getSymbolExtendedValue(*arg);
       converter.bindSymbol(*arg, hlfir::translateToExtendedValue(
-                                     loc, firOpBuilder, regionArgEntity,
-                                     regionArgEntity.isArray())
+                                     loc, firOpBuilder, hlfir::Entity{prv},
+                                     /*contiguousHint=*/
+                                     std::holds_alternative<fir::ArrayBoxValue>(
+                                         hostExV.matchee()))
                                      .first);
     }
 

>From d71bc1bcf07861c708a38fc75e106516fb6e17db Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Sun, 28 Apr 2024 23:57:20 -0500
Subject: [PATCH 09/11] Test  entities

---
 .../lib/Lower/OpenMP/DataSharingProcessor.cpp | 21 +++++++++---
 .../delayed-privatization-character.f90       | 33 +++++++++++++++++++
 2 files changed, 49 insertions(+), 5 deletions(-)
 create mode 100644 flang/test/Lower/OpenMP/delayed-privatization-character.f90

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 44422ea005460c..788e82dfd38425 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -425,12 +425,23 @@ void DataSharingProcessor::doPrivatize(
       mlir::Block *copyEntryBlock = firOpBuilder.createBlock(
           &copyRegion, /*insertPt=*/{}, {symType, symType}, {symLoc, symLoc});
       firOpBuilder.setInsertionPointToEnd(copyEntryBlock);
-      symTable->addSymbol(*sym,
-                          fir::substBase(symExV, copyRegion.getArgument(0)),
-                          /*force=*/true);
+
+      auto addSymbol = [&](unsigned argIdx, bool force = false) {
+        symExV.match(
+            [&](const fir::MutableBoxValue &box) {
+              symTable->addSymbol(
+                  *sym, fir::substBase(box, copyRegion.getArgument(argIdx)),
+                  force);
+            },
+            [&](const auto &box) {
+              symTable->addSymbol(*sym, copyRegion.getArgument(argIdx), force);
+            });
+      };
+
+      addSymbol(0, true);
       symTable->pushScope();
-      symTable->addSymbol(*sym,
-                          fir::substBase(symExV, copyRegion.getArgument(1)));
+      addSymbol(1);
+
       auto ip = firOpBuilder.saveInsertionPoint();
       copyFirstPrivateSymbol(sym, &ip);
 
diff --git a/flang/test/Lower/OpenMP/delayed-privatization-character.f90 b/flang/test/Lower/OpenMP/delayed-privatization-character.f90
new file mode 100644
index 00000000000000..5be594b259f2f2
--- /dev/null
+++ b/flang/test/Lower/OpenMP/delayed-privatization-character.f90
@@ -0,0 +1,33 @@
+! Test delayed privatization for the `CHARACTER` type.
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN:   -o - %s 2>&1 | FileCheck %s
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %s 2>&1 \
+! RUN:   | FileCheck %s
+
+subroutine delayed_privatization_character(var1, l)
+  implicit none
+  integer(8):: l
+  character(len = l)  :: var1
+
+!$omp parallel firstprivate(var1)
+  var1 = "test"
+!$omp end parallel
+end subroutine
+
+! CHECK-LABEL: omp.private {type = firstprivate}
+! CHECK-SAME: @[[PRIVATIZER_SYM:.*]] : [[TYPE:!fir.boxchar<1>]] alloc {
+
+! CHECK-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE]]):
+! CHECK-NEXT:   %[[UNBOX:.*]]:2 = fir.unboxchar %[[PRIV_ARG]]
+! CHECK:        %[[PRIV_ALLOC:.*]] = fir.alloca !fir.char<1,?>(%[[UNBOX]]#1 : index)
+! CHECK-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOC]] typeparams %[[UNBOX]]#1
+! CHECK-NEXT:   omp.yield(%[[PRIV_DECL]]#0 : !fir.boxchar<1>)
+
+! CHECK-NEXT: } copy {
+! CHECK-NEXT: ^bb0(%[[PRIV_ORIG_ARG:.*]]: [[TYPE]], %[[PRIV_PRIV_ARG:.*]]: [[TYPE]]):
+
+! CHECK-NEXT:   hlfir.assign %[[PRIV_ORIG_ARG]] to %[[PRIV_PRIV_ARG]]
+
+! CHECK-NEXT:   omp.yield(%[[PRIV_PRIV_ARG]] : !fir.boxchar<1>)
+! CHECK-NEXT: }

>From fb4d40b74c8f78e901af2b9c0005b52f6deb71af Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Mon, 29 Apr 2024 01:29:36 -0500
Subject: [PATCH 10/11] More tests for

---
 .../lib/Lower/OpenMP/DataSharingProcessor.cpp |  2 +-
 .../delayed-privatization-character.f90       | 56 ++++++++++++++-----
 2 files changed, 42 insertions(+), 16 deletions(-)

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 788e82dfd38425..61ed34f731dd0d 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -399,7 +399,7 @@ void DataSharingProcessor::doPrivatize(
 
       firOpBuilder.setInsertionPointToEnd(allocEntryBlock);
 
-      // TODO Delayed privatization has not been tested yet for: CharBoxValue,
+      // TODO Delayed privatization has not been tested yet for:
       // CharArrayBoxValue, BoxValue, or PolymorphicValue.
       fir::ExtendedValue localExV =
           hlfir::translateToExtendedValue(
diff --git a/flang/test/Lower/OpenMP/delayed-privatization-character.f90 b/flang/test/Lower/OpenMP/delayed-privatization-character.f90
index 5be594b259f2f2..db678ab13bbe69 100644
--- a/flang/test/Lower/OpenMP/delayed-privatization-character.f90
+++ b/flang/test/Lower/OpenMP/delayed-privatization-character.f90
@@ -1,10 +1,18 @@
 ! Test delayed privatization for the `CHARACTER` type.
 
+! RUN: split-file %s %t
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN:   -o - %t/dyn_len.f90 2>&1 | FileCheck %s --check-prefix=DYN_LEN
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %t/dyn_len.f90 2>&1 \
+! RUN:   | FileCheck %s --check-prefix=DYN_LEN
+
 ! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
-! RUN:   -o - %s 2>&1 | FileCheck %s
-! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %s 2>&1 \
-! RUN:   | FileCheck %s
+! RUN:   -o - %t/static_len.f90 2>&1 | FileCheck %s --check-prefix=STATIC_LEN
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %t/static_len.f90 2>&1 \
+! RUN:   | FileCheck %s --check-prefix=STATIC_LEN
 
+!--- dyn_len.f90
 subroutine delayed_privatization_character(var1, l)
   implicit none
   integer(8):: l
@@ -15,19 +23,37 @@ subroutine delayed_privatization_character(var1, l)
 !$omp end parallel
 end subroutine
 
-! CHECK-LABEL: omp.private {type = firstprivate}
-! CHECK-SAME: @[[PRIVATIZER_SYM:.*]] : [[TYPE:!fir.boxchar<1>]] alloc {
+! DYN_LEN-LABEL: omp.private {type = firstprivate}
+! DYN_LEN-SAME: @[[PRIVATIZER_SYM:.*]] : [[TYPE:!fir.boxchar<1>]] alloc {
+
+! DYN_LEN-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE]]):
+! DYN_LEN-NEXT:   %[[UNBOX:.*]]:2 = fir.unboxchar %[[PRIV_ARG]]
+! DYN_LEN:        %[[PRIV_ALLOC:.*]] = fir.alloca !fir.char<1,?>(%[[UNBOX]]#1 : index)
+! DYN_LEN-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOC]] typeparams %[[UNBOX]]#1
+! DYN_LEN-NEXT:   omp.yield(%[[PRIV_DECL]]#0 : !fir.boxchar<1>)
+
+! DYN_LEN-NEXT: } copy {
+! DYN_LEN-NEXT: ^bb0(%[[PRIV_ORIG_ARG:.*]]: [[TYPE]], %[[PRIV_PRIV_ARG:.*]]: [[TYPE]]):
+
+! DYN_LEN-NEXT:   hlfir.assign %[[PRIV_ORIG_ARG]] to %[[PRIV_PRIV_ARG]]
 
-! CHECK-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE]]):
-! CHECK-NEXT:   %[[UNBOX:.*]]:2 = fir.unboxchar %[[PRIV_ARG]]
-! CHECK:        %[[PRIV_ALLOC:.*]] = fir.alloca !fir.char<1,?>(%[[UNBOX]]#1 : index)
-! CHECK-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOC]] typeparams %[[UNBOX]]#1
-! CHECK-NEXT:   omp.yield(%[[PRIV_DECL]]#0 : !fir.boxchar<1>)
+! DYN_LEN-NEXT:   omp.yield(%[[PRIV_PRIV_ARG]] : !fir.boxchar<1>)
+! DYN_LEN-NEXT: }
 
-! CHECK-NEXT: } copy {
-! CHECK-NEXT: ^bb0(%[[PRIV_ORIG_ARG:.*]]: [[TYPE]], %[[PRIV_PRIV_ARG:.*]]: [[TYPE]]):
+!--- static_len.f90
+subroutine delayed_privatization_character_static_len(var1)
+  implicit none
+  character(len = 10)  :: var1
+
+!$omp parallel private(var1)
+  var1 = "test"
+!$omp end parallel
+end subroutine
 
-! CHECK-NEXT:   hlfir.assign %[[PRIV_ORIG_ARG]] to %[[PRIV_PRIV_ARG]]
+! STATIC_LEN-LABEL: omp.private {type = private}
+! STATIC_LEN-SAME: @[[PRIVATIZER_SYM:.*]] : [[TYPE:!fir.ref<!fir.char<1,10>>]] alloc {
 
-! CHECK-NEXT:   omp.yield(%[[PRIV_PRIV_ARG]] : !fir.boxchar<1>)
-! CHECK-NEXT: }
+! STATIC_LEN-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE]]):
+! STATIC_LEN-NEXT:   %[[C10:.*]] = arith.constant 10 : index
+! STATIC_LEN-NEXT:   %[[PRIV_ALLOC:.*]] = fir.alloca !fir.char<1,10>
+! STATIC_LEN-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOC]] typeparams %[[C10]]

>From 5578df24c6047fbb089781c14aedc7c9439726ae Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at amd.com>
Date: Mon, 29 Apr 2024 01:50:31 -0500
Subject: [PATCH 11/11] Add tests for character array

---
 .../lib/Lower/OpenMP/DataSharingProcessor.cpp |  4 +-
 .../delayed-privatization-character-array.f90 | 67 +++++++++++++++++++
 2 files changed, 69 insertions(+), 2 deletions(-)
 create mode 100644 flang/test/Lower/OpenMP/delayed-privatization-character-array.f90

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 61ed34f731dd0d..a352f1f5d9edf0 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -399,8 +399,8 @@ void DataSharingProcessor::doPrivatize(
 
       firOpBuilder.setInsertionPointToEnd(allocEntryBlock);
 
-      // TODO Delayed privatization has not been tested yet for:
-      // CharArrayBoxValue, BoxValue, or PolymorphicValue.
+      // TODO Delayed privatization has not been tested yet for: BoxValue, or
+      // PolymorphicValue.
       fir::ExtendedValue localExV =
           hlfir::translateToExtendedValue(
               symLoc, firOpBuilder, hlfir::Entity{allocRegion.getArgument(0)},
diff --git a/flang/test/Lower/OpenMP/delayed-privatization-character-array.f90 b/flang/test/Lower/OpenMP/delayed-privatization-character-array.f90
new file mode 100644
index 00000000000000..60230f53deab13
--- /dev/null
+++ b/flang/test/Lower/OpenMP/delayed-privatization-character-array.f90
@@ -0,0 +1,67 @@
+! Test delayed privatization for the `CHARACTER` array type.
+
+! RUN: split-file %s %t
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN:   -o - %t/static_len.f90 2>&1 | FileCheck %s --check-prefix=STATIC_LEN
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %t/static_len.f90 2>&1 \
+! RUN:   | FileCheck %s --check-prefix=STATIC_LEN
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp -mmlir --openmp-enable-delayed-privatization \
+! RUN:   -o - %t/dyn_len.f90 2>&1 | FileCheck %s --check-prefix=DYN_LEN
+! RUN: bbc -emit-hlfir -fopenmp --openmp-enable-delayed-privatization -o - %t/dyn_len.f90 2>&1 \
+! RUN:   | FileCheck %s --check-prefix=DYN_LEN
+
+--- static_len.f90
+subroutine delayed_privatization_character_array_static_len(var1)
+  implicit none
+  character(len = 10)  :: var1(5)
+
+!$omp parallel firstprivate(var1)
+  var1(1) = "test"
+!$omp end parallel
+end subroutine
+
+! STATIC_LEN-LABEL: omp.private {type = firstprivate}
+! STATIC_LEN-SAME: @[[PRIVATIZER_SYM:.*]] : [[TYPE:!fir.ref<!fir.array<5x!fir.char<1,10>>>]] alloc {
+
+! STATIC_LEN-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE]]):
+! STATIC_LEN-DAG:    %[[C5:.*]] = arith.constant 5 : index
+! STATIC_LEN-DAG:    %[[C10:.*]] = arith.constant 10 : index
+! STATIC_LEN-NEXT:   %[[PRIV_ALLOC:.*]] = fir.alloca !fir.array<5x!fir.char<1,10>>
+! STATIC_LEN-NEXT:   %[[ARRAY_SHAPE:.*]] = fir.shape %[[C5]]
+! STATIC_LEN-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOC]](%[[ARRAY_SHAPE]]) typeparams %[[C10]]
+! STATIC_LEN-NEXT:   omp.yield(%[[PRIV_DECL]]#0
+
+! STATIC_LEN-NEXT: } copy {
+! STATIC_LEN-NEXT: ^bb0(%[[PRIV_ORIG_ARG:.*]]: [[TYPE]], %[[PRIV_PRIV_ARG:.*]]: [[TYPE]]):
+! STATIC_LEN-NEXT:   hlfir.assign %[[PRIV_ORIG_ARG]] to %[[PRIV_PRIV_ARG]]
+
+! STATIC_LEN-NEXT:   omp.yield(%[[PRIV_PRIV_ARG]]
+! STATIC_LEN-NEXT: }
+
+!--- dyn_len.f90
+subroutine delayed_privatization_character_array_dynamic_len(var1, char_len, array_len)
+  implicit none
+  integer(8):: char_len
+  integer(8):: array_len
+  character(len = char_len)  :: var1(array_len)
+
+!$omp parallel private(var1)
+  var1(1) = "test"
+!$omp end parallel
+end subroutine
+
+! DYN_LEN-LABEL: omp.private {type = private}
+! DYN_LEN-SAME: @[[PRIVATIZER_SYM:.*]] : [[TYPE:!fir.box<!fir.array<\?x!fir.char<1,\?>>>]] alloc {
+
+! DYN_LEN-NEXT: ^bb0(%[[PRIV_ARG:.*]]: [[TYPE]]):
+
+! DYN_LEN:        %[[C0:.*]] = arith.constant 0 : index
+! DYN_LEN-NEXT:   %[[BOX_DIM:.*]]:3 = fir.box_dims %[[PRIV_ARG]], %[[C0]]
+! DYN_LEN:        %[[CHAR_LEN:.*]] = fir.box_elesize %[[PRIV_ARG]]
+! DYN_LEN-NEXT:   %[[PRIV_ALLOC:.*]] = fir.alloca !fir.array<?x!fir.char<1,?>>(%[[CHAR_LEN]] : index)
+! DYN_LEN-NEXT:   %[[ARRAY_SHAPE:.*]] = fir.shape
+! DYN_LEN-NEXT:   %[[PRIV_DECL:.*]]:2 = hlfir.declare %[[PRIV_ALLOC]](%[[ARRAY_SHAPE]]) typeparams %[[CHAR_LEN]]
+
+! DYN_LEN-NEXT:   omp.yield(%[[PRIV_DECL]]#0



More information about the flang-commits mailing list