Skip to content

[flang][MLIR][OpenMP] Extend delayed privatization for arrays and characters #85023

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 25 commits into from
May 4, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
24f62ad
[flang][MLIR][OpenMP] Extend delayed privatization for arrays
ergawy Mar 13, 2024
3740156
Merge remote-tracking branch 'upstream/main' into delayed_privatizati…
ergawy Apr 18, 2024
943c7e2
Use `genLboundsAndExtentsFromBox`.
ergawy Apr 18, 2024
81f422d
Add explicit todos for unsupported extended value types.
ergawy Apr 18, 2024
7cefbcb
Merge remote-tracking branch 'upstream/main' into delayed_privatizati…
ergawy Apr 18, 2024
df716da
Merge remote-tracking branch 'upstream/main' into delayed_privatizati…
ergawy Apr 24, 2024
738962b
Use `translateToExtendedValue`.
ergawy Apr 24, 2024
bd99c8f
Merge remote-tracking branch 'upstream/main' into delayed_privatizati…
ergawy Apr 25, 2024
080a285
clean-ups
ergawy Apr 25, 2024
0d13dbb
Extend uses for
ergawy Apr 25, 2024
05a68a6
Add one more test.
ergawy Apr 25, 2024
f6cb909
Merge remote-tracking branch 'upstream/main' into delayed_privatizati…
ergawy Apr 26, 2024
1692791
More clean-up
ergawy Apr 26, 2024
2283349
Merge remote-tracking branch 'upstream/main' into delayed_privatizati…
ergawy Apr 27, 2024
d71bc1b
Test entities
ergawy Apr 29, 2024
fe4de72
Merge remote-tracking branch 'upstream/main' into delayed_privatizati…
ergawy Apr 29, 2024
fb4d40b
More tests for
ergawy Apr 29, 2024
0d6aff2
Add tests for character array
ergawy Apr 29, 2024
c8e44e5
Merge remote-tracking branch 'upstream/main' into delayed_privatizati…
ergawy Apr 29, 2024
130dc7a
fix test error
ergawy Apr 29, 2024
9a706aa
Merge remote-tracking branch 'upstream/main' into delayed_privatizati…
ergawy May 3, 2024
5f8d398
Merge remote-tracking branch 'upstream/main' into delayed_privatizati…
ergawy May 3, 2024
c6a7cfa
use IsSimplyContiguous()
ergawy May 3, 2024
bb876c1
Merge remote-tracking branch 'upstream/main' into delayed_privatizati…
ergawy May 4, 2024
a30ceb3
Merge remote-tracking branch 'upstream/main' into delayed_privatizati…
ergawy May 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will need to handle other cases here (like the array character case (CharArrayBoxValue) and the case for non contiguous arrays (BoxValue)). You can make them explicit TODO here if you are not handling them/testing them in this patch.

An alternative would be to use hlfir::translateToExtendedValue here to get the new ExtendedValue and cover all cases (there are no cleanup returned if the input is an mlir::Value memory reference). But you would need to give it a small hint that the input fir.box is contiguous in the ArrayBoxValue and CharArrayBoxValue cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added explicit todos. I think using hlfir::translateToExtendedValue will not be possible here since we are dealing with the region argument of the privatizer's alloc region which is only an mlir::Value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An hlfir::Entity is just an mlir::Value with guarantees about what it is. I do not remember how the alloc region value is created, but if it is created from the first result of the original variable hlfir::declare, you can just do hlfir::Entity entity{allocRegion.getArgument(0)}`` and use hlfir::translateToExtendedValue`.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really appreciate your help. Indeed, after studying what is going on a bit more I now understand more what you mean. I am now using translateToExtendedValue as you suggested.

});

symTable->addSymbol(*sym, localExV);
symTable->pushScope();
cloneSymbol(sym);
firOpBuilder.create<mlir::omp::YieldOp>(
Expand Down
Original file line number Diff line number Diff line change
@@ -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: }
75 changes: 75 additions & 0 deletions flang/test/Lower/OpenMP/delayed-privatization-array.f90
Original file line number Diff line number Diff line change
@@ -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: }