Skip to content

[flang][OpenMP] Fix copyprivate allocatable/pointer lowering #95975

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.

Sign up for GitHub

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 1 commit into from
Jun 25, 2024
Merged

[flang][OpenMP] Fix copyprivate allocatable/pointer lowering #95975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 3 additions & 2 deletions flang/include/flang/Lower/AbstractConverter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "flang/Lower/LoweringOptions.h"
#include "flang/Lower/PFTDefs.h"
#include "flang/Optimizer/Builder/BoxValue.h"
#include "flang/Optimizer/Dialect/FIRAttr.h"
#include "flang/Semantics/symbol.h"
#include "mlir/IR/Builders.h"
#include "mlir/IR/BuiltinOps.h"
Expand Down Expand Up @@ -126,8 +127,8 @@ class AbstractConverter {
const Fortran::semantics::Symbol &sym,
mlir::OpBuilder::InsertPoint *copyAssignIP = nullptr) = 0;

virtual void copyVar(mlir::Location loc, mlir::Value dst,
mlir::Value src) = 0;
virtual void copyVar(mlir::Location loc, mlir::Value dst, mlir::Value src,
fir::FortranVariableFlagsEnum attrs) = 0;

/// For a given symbol, check if it is present in the inner-most
/// level of the symbol map.
Expand Down
52 changes: 33 additions & 19 deletions flang/lib/Lower/Bridge.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,10 +751,16 @@ class FirConverter : public Fortran::lower::AbstractConverter {
});
}

void copyVar(mlir::Location loc, mlir::Value dst,
mlir::Value src) override final {
void copyVar(mlir::Location loc, mlir::Value dst, mlir::Value src,
fir::FortranVariableFlagsEnum attrs) override final {
bool isAllocatable =
bitEnumContainsAny(attrs, fir::FortranVariableFlagsEnum::allocatable);
bool isPointer =
bitEnumContainsAny(attrs, fir::FortranVariableFlagsEnum::pointer);

copyVarHLFIR(loc, Fortran::lower::SymbolBox::Intrinsic{dst},
Fortran::lower::SymbolBox::Intrinsic{src});
Fortran::lower::SymbolBox::Intrinsic{src}, isAllocatable,
isPointer);
}

void copyHostAssociateVar(
Expand Down Expand Up @@ -1083,6 +1089,28 @@ class FirConverter : public Fortran::lower::AbstractConverter {
void copyVarHLFIR(mlir::Location loc, Fortran::lower::SymbolBox dst,
Fortran::lower::SymbolBox src) {
assert(lowerToHighLevelFIR());

bool isBoxAllocatable = dst.match(
[](const fir::MutableBoxValue &box) { return box.isAllocatable(); },
[](const fir::FortranVariableOpInterface &box) {
return fir::FortranVariableOpInterface(box).isAllocatable();
},
[](const auto &box) { return false; });

bool isBoxPointer = dst.match(
[](const fir::MutableBoxValue &box) { return box.isPointer(); },
[](const fir::FortranVariableOpInterface &box) {
return fir::FortranVariableOpInterface(box).isPointer();
},
[](const auto &box) { return false; });

copyVarHLFIR(loc, dst, src, isBoxAllocatable, isBoxPointer);
}

void copyVarHLFIR(mlir::Location loc, Fortran::lower::SymbolBox dst,
Fortran::lower::SymbolBox src, bool isAllocatable,
bool isPointer) {
assert(lowerToHighLevelFIR());
hlfir::Entity lhs{dst.getAddr()};
hlfir::Entity rhs{src.getAddr()};
// Temporary_lhs is set to true in hlfir.assign below to avoid user
Expand All @@ -1099,21 +1127,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
/*temporary_lhs=*/true);
};

bool isBoxAllocatable = dst.match(
[](const fir::MutableBoxValue &box) { return box.isAllocatable(); },
[](const fir::FortranVariableOpInterface &box) {
return fir::FortranVariableOpInterface(box).isAllocatable();
},
[](const auto &box) { return false; });

bool isBoxPointer = dst.match(
[](const fir::MutableBoxValue &box) { return box.isPointer(); },
[](const fir::FortranVariableOpInterface &box) {
return fir::FortranVariableOpInterface(box).isPointer();
},
[](const auto &box) { return false; });

if (isBoxAllocatable) {
if (isAllocatable) {
// Deep copy allocatable if it is allocated.
// Note that when allocated, the RHS is already allocated with the LHS
// shape for copy on entry in createHostAssociateVarClone.
Expand All @@ -1128,7 +1142,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
copyData(lhs, rhs);
})
.end();
} else if (isBoxPointer) {
} else if (isPointer) {
// Set LHS target to the target of RHS (do not copy the RHS
// target data into the LHS target storage).
auto loadVal = builder->create<fir::LoadOp>(loc, rhs);
Expand Down
2 changes: 1 addition & 1 deletion flang/lib/Lower/OpenMP/ClauseProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ createCopyFunc(mlir::Location loc, lower::AbstractConverter &converter,
auto declSrc = builder.create<hlfir::DeclareOp>(
loc, funcOp.getArgument(1), copyFuncName + "_src", shape, typeparams,
/*dummy_scope=*/nullptr, attrs);
converter.copyVar(loc, declDst.getBase(), declSrc.getBase());
converter.copyVar(loc, declDst.getBase(), declSrc.getBase(), varAttrs);
builder.create<mlir::func::ReturnOp>(loc);
return funcOp;
}
Expand Down
56 changes: 56 additions & 0 deletions flang/test/Lower/OpenMP/copyprivate2.f90
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
! Test lowering of COPYPRIVATE with allocatable/pointer variables.
! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s

!CHECK-LABEL: func private @_copy_box_ptr_i32(
!CHECK-SAME: %[[ARG0:.*]]: !fir.ref<!fir.box<!fir.ptr<i32>>>,
!CHECK-SAME: %[[ARG1:.*]]: !fir.ref<!fir.box<!fir.ptr<i32>>>) {
!CHECK-NEXT: %[[DST:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<pointer>,
!CHECK-SAME: uniq_name = "_copy_box_ptr_i32_dst"} : (!fir.ref<!fir.box<!fir.ptr<i32>>>) ->
!CHECK-SAME: (!fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<!fir.box<!fir.ptr<i32>>>)
!CHECK-NEXT: %[[SRC:.*]]:2 = hlfir.declare %[[ARG1]] {fortran_attrs = #fir.var_attrs<pointer>,
!CHECK-SAME: uniq_name = "_copy_box_ptr_i32_src"} : (!fir.ref<!fir.box<!fir.ptr<i32>>>) ->
!CHECK-SAME: (!fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<!fir.box<!fir.ptr<i32>>>)
!CHECK-NEXT: %[[SRC_VAL:.*]] = fir.load %[[SRC]]#0 : !fir.ref<!fir.box<!fir.ptr<i32>>>
!CHECK-NEXT: fir.store %[[SRC_VAL]] to %[[DST]]#0 : !fir.ref<!fir.box<!fir.ptr<i32>>>
!CHECK-NEXT: return
!CHECK-NEXT: }

!CHECK-LABEL: func private @_copy_box_heap_Uxi32(
!CHECK-SAME: %[[ARG0:.*]]: !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>,
!CHECK-SAME: %[[ARG1:.*]]: !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) {
!CHECK-NEXT: %[[DST:.*]]:2 = hlfir.declare %[[ARG0]] {fortran_attrs = #fir.var_attrs<allocatable>,
!CHECK-SAME: uniq_name = "_copy_box_heap_Uxi32_dst"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) ->
!CHECK-SAME: (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>)
!CHECK-NEXT: %[[SRC:.*]]:2 = hlfir.declare %[[ARG1]] {fortran_attrs = #fir.var_attrs<allocatable>,
!CHECK-SAME: uniq_name = "_copy_box_heap_Uxi32_src"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) ->
!CHECK-SAME: (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>)
!CHECK-NEXT: %[[DST_BOX:.*]] = fir.load %[[DST]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
!CHECK: fir.if %{{.*}} {
!CHECK-NEXT: %[[SRC_BOX:.*]] = fir.load %[[SRC]]#0 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
!CHECK-NEXT: hlfir.assign %[[SRC_BOX]] to %[[DST_BOX]] temporary_lhs : !fir.box<!fir.heap<!fir.array<?xi32>>>,
!CHECK-SAME: !fir.box<!fir.heap<!fir.array<?xi32>>>
!CHECK-NEXT: }
!CHECK-NEXT: return
!CHECK-NEXT: }

!CHECK-LABEL: func @_QPtest_alloc_ptr
!CHECK: omp.parallel {
!CHECK: %[[A:.*]]:2 = hlfir.declare %{{.*}} {fortran_attrs = #fir.var_attrs<allocatable>,
!CHECK-SAME: uniq_name = "_QFtest_alloc_ptrEa"} : (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) ->
!CHECK-SAME: (!fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>, !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>)
!CHECK: %[[P:.*]]:2 = hlfir.declare %{{.*}} {fortran_attrs = #fir.var_attrs<pointer>,
!CHECK-SAME: uniq_name = "_QFtest_alloc_ptrEp"} : (!fir.ref<!fir.box<!fir.ptr<i32>>>) ->
!CHECK-SAME: (!fir.ref<!fir.box<!fir.ptr<i32>>>, !fir.ref<!fir.box<!fir.ptr<i32>>>)
!CHECK: omp.single copyprivate(
!CHECK-SAME: %[[A]]#0 -> @_copy_box_heap_Uxi32 : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>,
!CHECK-SAME: %[[P]]#0 -> @_copy_box_ptr_i32 : !fir.ref<!fir.box<!fir.ptr<i32>>>)
!CHEK: }
subroutine test_alloc_ptr()
integer, allocatable :: a(:)
integer, pointer :: p

!$omp parallel private(a, p)
!$omp single
!$omp end single copyprivate(a, p)
!$omp end parallel
end subroutine
Loading