diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp index 90644279d9e78..5c29c78141730 100644 --- a/flang/lib/Lower/OpenACC.cpp +++ b/flang/lib/Lower/OpenACC.cpp @@ -336,7 +336,7 @@ static void genDeclareDataOperandOperationsWithModifier( template static void genDataExitOperations(fir::FirOpBuilder &builder, llvm::SmallVector operands, - bool structured, bool implicit) { + bool structured) { for (mlir::Value operand : operands) { auto entryOp = mlir::dyn_cast_or_null(operand.getDefiningOp()); assert(entryOp && "data entry op expected"); @@ -346,7 +346,7 @@ static void genDataExitOperations(fir::FirOpBuilder &builder, varPtr = entryOp.getVarPtr(); builder.create(entryOp.getLoc(), entryOp.getAccPtr(), varPtr, entryOp.getBounds(), entryOp.getDataClause(), - structured, implicit, + structured, entryOp.getImplicit(), builder.getStringAttr(*entryOp.getName())); } } @@ -1872,9 +1872,24 @@ createComputeOp(Fortran::lower::AbstractConverter &converter, } else if (const auto *reductionClause = std::get_if( &clause.u)) { - if (!outerCombined) + // A reduction clause on a combined construct is treated as if it appeared + // on the loop construct. So don't generate a reduction clause when it is + // combined - delay it to the loop. However, a reduction clause on a + // combined construct implies a copy clause so issue an implicit copy + // instead. + if (!outerCombined) { genReductions(reductionClause->v, converter, semanticsContext, stmtCtx, reductionOperands, reductionRecipes); + } else { + auto crtDataStart = dataClauseOperands.size(); + genDataOperandOperations( + std::get(reductionClause->v.t), + converter, semanticsContext, stmtCtx, dataClauseOperands, + mlir::acc::DataClause::acc_reduction, + /*structured=*/true, /*implicit=*/true); + copyEntryOperands.append(dataClauseOperands.begin() + crtDataStart, + dataClauseOperands.end()); + } } else if (const auto *defaultClause = std::get_if( &clause.u)) { @@ -1944,13 +1959,13 @@ createComputeOp(Fortran::lower::AbstractConverter &converter, // Create the exit operations after the region. genDataExitOperations( - builder, copyEntryOperands, /*structured=*/true, /*implicit=*/false); + builder, copyEntryOperands, /*structured=*/true); genDataExitOperations( - builder, copyoutEntryOperands, /*structured=*/true, /*implicit=*/false); + builder, copyoutEntryOperands, /*structured=*/true); genDataExitOperations( - builder, attachEntryOperands, /*structured=*/true, /*implicit=*/false); + builder, attachEntryOperands, /*structured=*/true); genDataExitOperations( - builder, createEntryOperands, /*structured=*/true, /*implicit=*/false); + builder, createEntryOperands, /*structured=*/true); builder.restoreInsertionPoint(insPt); return computeOp; @@ -2099,13 +2114,13 @@ static void genACCDataOp(Fortran::lower::AbstractConverter &converter, // Create the exit operations after the region. genDataExitOperations( - builder, copyEntryOperands, /*structured=*/true, /*implicit=*/false); + builder, copyEntryOperands, /*structured=*/true); genDataExitOperations( - builder, copyoutEntryOperands, /*structured=*/true, /*implicit=*/false); + builder, copyoutEntryOperands, /*structured=*/true); genDataExitOperations( - builder, attachEntryOperands, /*structured=*/true, /*implicit=*/false); + builder, attachEntryOperands, /*structured=*/true); genDataExitOperations( - builder, createEntryOperands, /*structured=*/true, /*implicit=*/false); + builder, createEntryOperands, /*structured=*/true); builder.restoreInsertionPoint(insPt); } @@ -2415,11 +2430,11 @@ genACCExitDataOp(Fortran::lower::AbstractConverter &converter, exitDataOp.setFinalizeAttr(builder.getUnitAttr()); genDataExitOperations( - builder, copyoutOperands, /*structured=*/false, /*implicit=*/false); + builder, copyoutOperands, /*structured=*/false); genDataExitOperations( - builder, deleteOperands, /*structured=*/false, /*implicit=*/false); + builder, deleteOperands, /*structured=*/false); genDataExitOperations( - builder, detachOperands, /*structured=*/false, /*implicit=*/false); + builder, detachOperands, /*structured=*/false); } template @@ -2594,7 +2609,7 @@ genACCUpdateOp(Fortran::lower::AbstractConverter &converter, builder, currentLocation, operands, operandSegments); genDataExitOperations( - builder, updateHostOperands, /*structured=*/false, /*implicit=*/false); + builder, updateHostOperands, /*structured=*/false); if (addAsyncAttr) updateOp.setAsyncAttr(builder.getUnitAttr()); @@ -3054,17 +3069,14 @@ genDeclareInFunction(Fortran::lower::AbstractConverter &converter, builder.setInsertionPointAfter(declareOp); } genDataExitOperations( - builder, createEntryOperands, /*structured=*/true, - /*implicit=*/false); + builder, createEntryOperands, /*structured=*/true); genDataExitOperations( - builder, deviceResidentEntryOperands, /*structured=*/true, - /*implicit=*/false); + builder, deviceResidentEntryOperands, /*structured=*/true); genDataExitOperations( - builder, copyEntryOperands, /*structured=*/true, /*implicit=*/false); + builder, copyEntryOperands, /*structured=*/true); genDataExitOperations( - builder, copyoutEntryOperands, /*structured=*/true, - /*implicit=*/false); + builder, copyoutEntryOperands, /*structured=*/true); }); } diff --git a/flang/test/Lower/OpenACC/acc-kernels-loop.f90 b/flang/test/Lower/OpenACC/acc-kernels-loop.f90 index e708ca65e7c14..8679e5f6ccd5f 100644 --- a/flang/test/Lower/OpenACC/acc-kernels-loop.f90 +++ b/flang/test/Lower/OpenACC/acc-kernels-loop.f90 @@ -751,12 +751,16 @@ subroutine acc_kernels_loop reduction_i = 1 end do -! CHECK: acc.kernels { +! CHECK: %[[COPYINREDR:.*]] = acc.copyin varPtr(%{{.*}} : !fir.ref) -> !fir.ref {dataClause = #acc, implicit = true, name = "reduction_r"} +! CHECK: %[[COPYINREDI:.*]] = acc.copyin varPtr(%{{.*}} : !fir.ref) -> !fir.ref {dataClause = #acc, implicit = true, name = "reduction_i"} +! CHECK: acc.kernels dataOperands(%[[COPYINREDR]], %[[COPYINREDI]] : !fir.ref, !fir.ref) { ! CHECK: acc.loop reduction(@reduction_add_ref_f32 -> %{{.*}} : !fir.ref, @reduction_mul_ref_i32 -> %{{.*}} : !fir.ref) { ! CHECK: fir.do_loop ! CHECK: acc.yield ! CHECK-NEXT: }{{$}} ! CHECK: acc.terminator ! CHECK-NEXT: }{{$}} +! CHECK: acc.copyout accPtr(%[[COPYINREDR]] : !fir.ref) to varPtr(%{{.*}} : !fir.ref) {dataClause = #acc, implicit = true, name = "reduction_r"} +! CHECK: acc.copyout accPtr(%[[COPYINREDI]] : !fir.ref) to varPtr(%{{.*}} : !fir.ref) {dataClause = #acc, implicit = true, name = "reduction_i"} end subroutine diff --git a/flang/test/Lower/OpenACC/acc-parallel-loop.f90 b/flang/test/Lower/OpenACC/acc-parallel-loop.f90 index eea4950b6d38f..e0a29273bd783 100644 --- a/flang/test/Lower/OpenACC/acc-parallel-loop.f90 +++ b/flang/test/Lower/OpenACC/acc-parallel-loop.f90 @@ -770,13 +770,17 @@ subroutine acc_parallel_loop reduction_i = 1 end do -! CHECK: acc.parallel { +! CHECK: %[[COPYINREDR:.*]] = acc.copyin varPtr(%{{.*}} : !fir.ref) -> !fir.ref {dataClause = #acc, implicit = true, name = "reduction_r"} +! CHECK: %[[COPYINREDI:.*]] = acc.copyin varPtr(%{{.*}} : !fir.ref) -> !fir.ref {dataClause = #acc, implicit = true, name = "reduction_i"} +! CHECK: acc.parallel dataOperands(%[[COPYINREDR]], %[[COPYINREDI]] : !fir.ref, !fir.ref) { ! CHECK: acc.loop reduction(@reduction_add_ref_f32 -> %{{.*}} : !fir.ref, @reduction_mul_ref_i32 -> %{{.*}} : !fir.ref) { ! CHECK: fir.do_loop ! CHECK: acc.yield ! CHECK-NEXT: }{{$}} ! CHECK: acc.yield ! CHECK-NEXT: }{{$}} +! CHECK: acc.copyout accPtr(%[[COPYINREDR]] : !fir.ref) to varPtr(%{{.*}} : !fir.ref) {dataClause = #acc, implicit = true, name = "reduction_r"} +! CHECK: acc.copyout accPtr(%[[COPYINREDI]] : !fir.ref) to varPtr(%{{.*}} : !fir.ref) {dataClause = #acc, implicit = true, name = "reduction_i"} !$acc parallel loop do 10 i=0, n diff --git a/flang/test/Lower/OpenACC/acc-serial-loop.f90 b/flang/test/Lower/OpenACC/acc-serial-loop.f90 index fb7e3615b698c..c94b5a577350a 100644 --- a/flang/test/Lower/OpenACC/acc-serial-loop.f90 +++ b/flang/test/Lower/OpenACC/acc-serial-loop.f90 @@ -705,12 +705,16 @@ subroutine acc_serial_loop reduction_i = 1 end do -! CHECK: acc.serial { +! CHECK: %[[COPYINREDR:.*]] = acc.copyin varPtr(%{{.*}} : !fir.ref) -> !fir.ref {dataClause = #acc, implicit = true, name = "reduction_r"} +! CHECK: %[[COPYINREDI:.*]] = acc.copyin varPtr(%{{.*}} : !fir.ref) -> !fir.ref {dataClause = #acc, implicit = true, name = "reduction_i"} +! CHECK: acc.serial dataOperands(%[[COPYINREDR]], %[[COPYINREDI]] : !fir.ref, !fir.ref) { ! CHECK: acc.loop reduction(@reduction_add_ref_f32 -> %{{.*}} : !fir.ref, @reduction_mul_ref_i32 -> %{{.*}} : !fir.ref) { ! CHECK: fir.do_loop ! CHECK: acc.yield ! CHECK-NEXT: }{{$}} ! CHECK: acc.yield ! CHECK-NEXT: }{{$}} +! CHECK: acc.copyout accPtr(%[[COPYINREDR]] : !fir.ref) to varPtr(%{{.*}} : !fir.ref) {dataClause = #acc, implicit = true, name = "reduction_r"} +! CHECK: acc.copyout accPtr(%[[COPYINREDI]] : !fir.ref) to varPtr(%{{.*}} : !fir.ref) {dataClause = #acc, implicit = true, name = "reduction_i"} end subroutine acc_serial_loop diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp index b7e2aec6a4e6a..98c800033cbe9 100644 --- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp +++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp @@ -131,7 +131,8 @@ LogicalResult acc::CopyinOp::verify() { // Test for all clauses this operation can be decomposed from: if (!getImplicit() && getDataClause() != acc::DataClause::acc_copyin && getDataClause() != acc::DataClause::acc_copyin_readonly && - getDataClause() != acc::DataClause::acc_copy) + getDataClause() != acc::DataClause::acc_copy && + getDataClause() != acc::DataClause::acc_reduction) return emitError( "data clause associated with copyin operation must match its intent" " or specify original clause this operation was decomposed from"); @@ -212,7 +213,8 @@ LogicalResult acc::CopyoutOp::verify() { // Test for all clauses this operation can be decomposed from: if (getDataClause() != acc::DataClause::acc_copyout && getDataClause() != acc::DataClause::acc_copyout_zero && - getDataClause() != acc::DataClause::acc_copy) + getDataClause() != acc::DataClause::acc_copy && + getDataClause() != acc::DataClause::acc_reduction) return emitError( "data clause associated with copyout operation must match its intent" " or specify original clause this operation was decomposed from");