[flang-commits] [flang] [flang][openacc] Fix post_alloc declare function ordering (PR #69980)
Razvan Lupusoru via flang-commits
flang-commits at lists.llvm.org
Mon Oct 23 16:05:19 PDT 2023
https://github.com/razvanlupusoru created https://github.com/llvm/llvm-project/pull/69980
The declare actions were introduced to capture semantics dealing with allocation of descriptor-based variable. However, the post_alloc action has an ordering error. It needs to update descriptor first before the mapping action of the data. The reason for this is that implicit attach must occur during mapping action - but updating the descriptor synchronizes it with the host copy (which would hold a host pointer).
>From 91d8751b7047e4922cd513255508d51306ac8972 Mon Sep 17 00:00:00 2001
From: Razvan Lupusoru <rlupusoru at nvidia.com>
Date: Mon, 23 Oct 2023 16:03:37 -0700
Subject: [PATCH] [flang][openacc] Fix post_alloc declare function ordering
The declare actions were introduced to capture semantics dealing with
allocation of descriptor-based variable. However, the post_alloc
action has an ordering error. It needs to update descriptor first
before the mapping action of the data. The reason for this is that
implicit attach must occur during mapping action - but updating
the descriptor synchronizes it with the host copy (which would hold
a host pointer).
---
flang/lib/Lower/OpenACC.cpp | 56 ++++++++++++++----------
flang/test/Lower/OpenACC/acc-declare.f90 | 16 +++----
2 files changed, 41 insertions(+), 31 deletions(-)
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index 4fafcebc30d116a..1dffadace6d5fa8 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -37,6 +37,7 @@ static unsigned routineCounter = 0;
static constexpr llvm::StringRef accRoutinePrefix = "acc_routine_";
static constexpr llvm::StringRef accPrivateInitName = "acc.private.init";
static constexpr llvm::StringRef accReductionInitName = "acc.reduction.init";
+static constexpr llvm::StringRef accFirDescriptorPostfix = "_desc";
static mlir::Location
genOperandLocation(Fortran::lower::AbstractConverter &converter,
@@ -138,27 +139,31 @@ static void createDeclareAllocFuncWithArg(mlir::OpBuilder &modBuilder,
auto registerFuncOp = createDeclareFunc(
modBuilder, builder, loc, registerFuncName.str(), {descTy}, {loc});
+ llvm::SmallVector<mlir::Value> bounds;
+ std::stringstream asFortranDesc;
+ asFortranDesc << asFortran.str() << accFirDescriptorPostfix.str();
+
+ // Updating descriptor must occur before the mapping of the data so that
+ // attached data pointer is not overwritten.
+ mlir::acc::UpdateDeviceOp updateDeviceOp =
+ createDataEntryOp<mlir::acc::UpdateDeviceOp>(
+ builder, loc, registerFuncOp.getArgument(0), asFortranDesc, bounds,
+ /*structured=*/false, /*implicit=*/true,
+ mlir::acc::DataClause::acc_update_device, descTy);
+ llvm::SmallVector<int32_t> operandSegments{0, 0, 0, 0, 0, 1};
+ llvm::SmallVector<mlir::Value> operands{updateDeviceOp.getResult()};
+ createSimpleOp<mlir::acc::UpdateOp>(builder, loc, operands, operandSegments);
+
mlir::Value desc =
builder.create<fir::LoadOp>(loc, registerFuncOp.getArgument(0));
fir::BoxAddrOp boxAddrOp = builder.create<fir::BoxAddrOp>(loc, desc);
addDeclareAttr(builder, boxAddrOp.getOperation(), clause);
-
- llvm::SmallVector<mlir::Value> bounds;
EntryOp entryOp = createDataEntryOp<EntryOp>(
builder, loc, boxAddrOp.getResult(), asFortran, bounds,
/*structured=*/false, /*implicit=*/false, clause, boxAddrOp.getType());
builder.create<mlir::acc::DeclareEnterOp>(
loc, mlir::ValueRange(entryOp.getAccPtr()));
- asFortran << "_desc";
- mlir::acc::UpdateDeviceOp updateDeviceOp =
- createDataEntryOp<mlir::acc::UpdateDeviceOp>(
- builder, loc, registerFuncOp.getArgument(0), asFortran, bounds,
- /*structured=*/false, /*implicit=*/true,
- mlir::acc::DataClause::acc_update_device, descTy);
- llvm::SmallVector<int32_t> operandSegments{0, 0, 0, 0, 0, 1};
- llvm::SmallVector<mlir::Value> operands{updateDeviceOp.getResult()};
- createSimpleOp<mlir::acc::UpdateOp>(builder, loc, operands, operandSegments);
modBuilder.setInsertionPointAfter(registerFuncOp);
builder.restoreInsertionPoint(crtInsPt);
}
@@ -208,7 +213,7 @@ static void createDeclareDeallocFuncWithArg(
auto postDeallocOp = createDeclareFunc(
modBuilder, builder, loc, postDeallocFuncName.str(), {descTy}, {loc});
loadOp = builder.create<fir::LoadOp>(loc, postDeallocOp.getArgument(0));
- asFortran << "_desc";
+ asFortran << accFirDescriptorPostfix.str();
mlir::acc::UpdateDeviceOp updateDeviceOp =
createDataEntryOp<mlir::acc::UpdateDeviceOp>(
builder, loc, loadOp, asFortran, bounds,
@@ -2753,28 +2758,33 @@ static void createDeclareAllocFunc(mlir::OpBuilder &modBuilder,
fir::AddrOfOp addrOp = builder.create<fir::AddrOfOp>(
loc, fir::ReferenceType::get(globalOp.getType()), globalOp.getSymbol());
- auto loadOp = builder.create<fir::LoadOp>(loc, addrOp.getResult());
- fir::BoxAddrOp boxAddrOp = builder.create<fir::BoxAddrOp>(loc, loadOp);
- addDeclareAttr(builder, boxAddrOp.getOperation(), clause);
std::stringstream asFortran;
asFortran << Fortran::lower::mangle::demangleName(globalOp.getSymName());
+ std::stringstream asFortranDesc;
+ asFortranDesc << asFortran.str() << accFirDescriptorPostfix.str();
llvm::SmallVector<mlir::Value> bounds;
- EntryOp entryOp = createDataEntryOp<EntryOp>(
- builder, loc, boxAddrOp.getResult(), asFortran, bounds,
- /*structured=*/false, /*implicit=*/false, clause, boxAddrOp.getType());
- builder.create<mlir::acc::DeclareEnterOp>(
- loc, mlir::ValueRange(entryOp.getAccPtr()));
- asFortran << "_desc";
+ // Updating descriptor must occur before the mapping of the data so that
+ // attached data pointer is not overwritten.
mlir::acc::UpdateDeviceOp updateDeviceOp =
createDataEntryOp<mlir::acc::UpdateDeviceOp>(
- builder, loc, addrOp, asFortran, bounds,
+ builder, loc, addrOp, asFortranDesc, bounds,
/*structured=*/false, /*implicit=*/true,
mlir::acc::DataClause::acc_update_device, addrOp.getType());
llvm::SmallVector<int32_t> operandSegments{0, 0, 0, 0, 0, 1};
llvm::SmallVector<mlir::Value> operands{updateDeviceOp.getResult()};
createSimpleOp<mlir::acc::UpdateOp>(builder, loc, operands, operandSegments);
+
+ auto loadOp = builder.create<fir::LoadOp>(loc, addrOp.getResult());
+ fir::BoxAddrOp boxAddrOp = builder.create<fir::BoxAddrOp>(loc, loadOp);
+ addDeclareAttr(builder, boxAddrOp.getOperation(), clause);
+ EntryOp entryOp = createDataEntryOp<EntryOp>(
+ builder, loc, boxAddrOp.getResult(), asFortran, bounds,
+ /*structured=*/false, /*implicit=*/false, clause, boxAddrOp.getType());
+ builder.create<mlir::acc::DeclareEnterOp>(
+ loc, mlir::ValueRange(entryOp.getAccPtr()));
+
modBuilder.setInsertionPointAfter(registerFuncOp);
}
@@ -2832,7 +2842,7 @@ static void createDeclareDeallocFunc(mlir::OpBuilder &modBuilder,
addrOp = builder.create<fir::AddrOfOp>(
loc, fir::ReferenceType::get(globalOp.getType()), globalOp.getSymbol());
- asFortran << "_desc";
+ asFortran << accFirDescriptorPostfix.str();
mlir::acc::UpdateDeviceOp updateDeviceOp =
createDataEntryOp<mlir::acc::UpdateDeviceOp>(
builder, loc, addrOp, asFortran, bounds,
diff --git a/flang/test/Lower/OpenACC/acc-declare.f90 b/flang/test/Lower/OpenACC/acc-declare.f90
index 11f62f1c24dfa86..f77e5489a61578f 100644
--- a/flang/test/Lower/OpenACC/acc-declare.f90
+++ b/flang/test/Lower/OpenACC/acc-declare.f90
@@ -228,12 +228,12 @@ subroutine acc_declare_allocate()
! CHECK-LABEL: func.func private @_QMacc_declareFacc_declare_allocateEa_acc_declare_update_desc_post_alloc(
! CHECK-SAME: %[[ARG0:.*]]: !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) {
+! CHECK: %[[UPDATE:.*]] = acc.update_device varPtr(%[[ARG0]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) -> !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>> {implicit = true, name = "a_desc", structured = false}
+! CHECK: acc.update dataOperands(%[[UPDATE]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>)
! CHECK: %[[LOAD:.*]] = fir.load %[[ARG0]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
! CHECK: %[[BOX_ADDR:.*]] = fir.box_addr %[[LOAD]] {acc.declare = #acc.declare<dataClause = acc_create>} : (!fir.box<!fir.heap<!fir.array<?xi32>>>) -> !fir.heap<!fir.array<?xi32>>
! CHECK: %[[CREATE:.*]] = acc.create varPtr(%[[BOX_ADDR]] : !fir.heap<!fir.array<?xi32>>) -> !fir.heap<!fir.array<?xi32>> {name = "a", structured = false}
! CHECK: acc.declare_enter dataOperands(%[[CREATE]] : !fir.heap<!fir.array<?xi32>>)
-! CHECK: %[[UPDATE:.*]] = acc.update_device varPtr(%[[ARG0]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) -> !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>> {implicit = true, name = "a_desc", structured = false}
-! CHECK: acc.update dataOperands(%[[UPDATE]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>)
! CHECK: return
! CHECK: }
@@ -241,9 +241,9 @@ subroutine acc_declare_allocate()
! CHECK-SAME: %[[ARG0:.*]]: !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) {
! CHECK: %[[LOAD:.*]] = fir.load %[[ARG0]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
! CHECK: %[[BOX_ADDR:.*]] = fir.box_addr %[[LOAD]] {acc.declare = #acc.declare<dataClause = acc_create>} : (!fir.box<!fir.heap<!fir.array<?xi32>>>) -> !fir.heap<!fir.array<?xi32>>
-! CHECK: %[[GETDEVICEPTR:.*]] = acc.getdeviceptr varPtr(%[[BOX_ADDR]] : !fir.heap<!fir.array<?xi32>>) -> !fir.heap<!fir.array<?xi32>> {dataClause = #acc<data_clause acc_create>, name = "a_desc", structured = false}
+! CHECK: %[[GETDEVICEPTR:.*]] = acc.getdeviceptr varPtr(%[[BOX_ADDR]] : !fir.heap<!fir.array<?xi32>>) -> !fir.heap<!fir.array<?xi32>> {dataClause = #acc<data_clause acc_create>, name = "a", structured = false}
! CHECK: acc.declare_exit dataOperands(%[[GETDEVICEPTR]] : !fir.heap<!fir.array<?xi32>>)
-! CHECK: acc.delete accPtr(%[[GETDEVICEPTR]] : !fir.heap<!fir.array<?xi32>>) {dataClause = #acc<data_clause acc_create>, name = "a_desc", structured = false}
+! CHECK: acc.delete accPtr(%[[GETDEVICEPTR]] : !fir.heap<!fir.array<?xi32>>) {dataClause = #acc<data_clause acc_create>, name = "a", structured = false}
! CHECK: return
! CHECK: }
@@ -251,7 +251,7 @@ subroutine acc_declare_allocate()
! CHECK-SAME: %[[ARG0:.*]]: !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) {
! CHECK: %[[LOAD:.*]] = fir.load %[[ARG0]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
! CHECK: %[[BOX_ADDR:.*]] = fir.box_addr %[[LOAD]] : (!fir.box<!fir.heap<!fir.array<?xi32>>>) -> !fir.heap<!fir.array<?xi32>>
-! CHECK: %[[UPDATE:.*]] = acc.update_device varPtr(%[[BOX_ADDR]] : !fir.heap<!fir.array<?xi32>>) -> !fir.heap<!fir.array<?xi32>> {implicit = true, name = "a_desc_desc", structured = false}
+! CHECK: %[[UPDATE:.*]] = acc.update_device varPtr(%[[BOX_ADDR]] : !fir.heap<!fir.array<?xi32>>) -> !fir.heap<!fir.array<?xi32>> {implicit = true, name = "a_desc", structured = false}
! CHECK: acc.update dataOperands(%[[UPDATE]] : !fir.heap<!fir.array<?xi32>>)
! CHECK: return
! CHECK: }
@@ -312,18 +312,18 @@ module acc_declare_allocatable_test
! CHECK-LABEL: func.func private @_QMacc_declare_allocatable_testEdata1_acc_declare_update_desc_post_alloc() {
! CHECK: %[[GLOBAL_ADDR:.*]] = fir.address_of(@_QMacc_declare_allocatable_testEdata1) : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
+! CHECK: %[[UPDATE:.*]] = acc.update_device varPtr(%[[GLOBAL_ADDR]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) -> !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>> {implicit = true, name = "data1_desc", structured = false}
+! CHECK: acc.update dataOperands(%[[UPDATE]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>)
! CHECK: %[[LOAD:.*]] = fir.load %[[GLOBAL_ADDR]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
! CHECK: %[[BOXADDR:.*]] = fir.box_addr %[[LOAD]] {acc.declare = #acc.declare<dataClause = acc_create>} : (!fir.box<!fir.heap<!fir.array<?xi32>>>) -> !fir.heap<!fir.array<?xi32>>
! CHECK: %[[CREATE:.*]] = acc.create varPtr(%[[BOXADDR]] : !fir.heap<!fir.array<?xi32>>) -> !fir.heap<!fir.array<?xi32>> {name = "data1", structured = false}
! CHECK: acc.declare_enter dataOperands(%[[CREATE]] : !fir.heap<!fir.array<?xi32>>)
-! CHECK: %[[UPDATE:.*]] = acc.update_device varPtr(%[[GLOBAL_ADDR]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>) -> !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>> {implicit = true, name = "data1_desc", structured = false}
-! CHECK: acc.update dataOperands(%[[UPDATE]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>)
! CHECK: return
! CHECK: }
! CHECK-LABEL: func.func private @_QMacc_declare_allocatable_testEdata1_acc_declare_update_desc_pre_dealloc() {
! CHECK: %[[GLOBAL_ADDR:.*]] = fir.address_of(@_QMacc_declare_allocatable_testEdata1) : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
-! CHECK: %[[LOAD]] = fir.load %[[GLOBAL_ADDR]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
+! CHECK: %[[LOAD:.*]] = fir.load %[[GLOBAL_ADDR]] : !fir.ref<!fir.box<!fir.heap<!fir.array<?xi32>>>>
! CHECK: %[[BOXADDR:.*]] = fir.box_addr %[[LOAD]] {acc.declare = #acc.declare<dataClause = acc_create>} : (!fir.box<!fir.heap<!fir.array<?xi32>>>) -> !fir.heap<!fir.array<?xi32>>
! CHECK: %[[DEVPTR:.*]] = acc.getdeviceptr varPtr(%[[BOXADDR]] : !fir.heap<!fir.array<?xi32>>) -> !fir.heap<!fir.array<?xi32>> {dataClause = #acc<data_clause acc_create>, name = "data1", structured = false}
! CHECK: acc.declare_exit dataOperands(%[[DEVICEPTR]] : !fir.heap<!fir.array<?xi32>>)
More information about the flang-commits
mailing list