[flang-commits] [flang] [flang][MLIR][OpenMP] Extend delayed privatization for arrays (PR #85023)
Kareem Ergawy via flang-commits
flang-commits at lists.llvm.org
Wed Apr 24 21:56:36 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 1/6] [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 2/6] 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 3/6] 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 4/6] 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 5/6] 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 6/6] 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);
More information about the flang-commits
mailing list