diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp --- a/flang/lib/Lower/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP.cpp @@ -90,9 +90,11 @@ } template -static bool privatizeVars(Op &op, Fortran::lower::AbstractConverter &converter, - const Fortran::parser::OmpClauseList &opClauseList, - Fortran::lower::pft::Evaluation &eval) { +static bool +privatizeVars(Op &op, Fortran::lower::AbstractConverter &converter, + const Fortran::parser::OmpClauseList &opClauseList, + Fortran::lower::pft::Evaluation &eval, + const Fortran::parser::OmpClauseList *otherClauses = nullptr) { fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); auto insPt = firOpBuilder.saveInsertionPoint(); // Symbols in private, firstprivate, and/or lastprivate clauses. @@ -120,8 +122,52 @@ } else if (const auto &lastPrivateClause = std::get_if( &clause.u)) { - // TODO: Add lastprivate support for sections construct, simd construct - if (std::is_same_v) { + // TODO: Add lastprivate support for simd construct + if (std::is_same_v) { + if (&eval == &eval.parentConstruct->getLastNestedEvaluation()) { + // For `omp.sections`, lastprivatized variables occur in + // lexically final `omp.section` operation. The following FIR + // shall be generated for the same: + // + // omp.sections lastprivate(...) { + // omp.section {...} + // omp.section {...} + // omp.section { + // fir.allocate for `private`/`firstprivate` + // + // scf.if %true { + // ^%lpv_update_blk + // } + // } + // } + // + // To keep code consistency while handling privatization + // through this control flow, add a `scf.if` operation + // that always evaluates to true, in order to create + // a dedicated sub-region in `omp.section` where + // lastprivate FIR can reside. Later canonicalizations + // will optimize away this operation. + + omp::SectionOp *sectionOp = dyn_cast(&op); + mlir::scf::IfOp ifOp = firOpBuilder.create( + sectionOp->getLoc(), + firOpBuilder.createIntegerConstant( + sectionOp->getLoc(), firOpBuilder.getIntegerType(1), 0x1), + /*else*/ false); + firOpBuilder.setInsertionPointToStart(&ifOp.getThenRegion().front()); + if (otherClauses) + for (const Fortran::parser::OmpClause &otherClause : + otherClauses->v) + if (std::get_if( + &otherClause.u)) + firOpBuilder.create( + converter.getCurrentLocation()); + firOpBuilder.setInsertionPointToStart(&ifOp.getThenRegion().front()); + lastPrivIP = firOpBuilder.saveInsertionPoint(); + firOpBuilder.setInsertionPoint(ifOp); + insPt = firOpBuilder.saveInsertionPoint(); + } + } else if (std::is_same_v) { omp::WsLoopOp *wsLoopOp = dyn_cast(&op); mlir::Operation *lastOper = wsLoopOp->getRegion().back().getTerminator(); @@ -464,13 +510,17 @@ //// region. /// \param [in] outerCombined - is this an outer operation - prevents /// privatization. +/// \param [in] otherClauses - should `op` have more than one clause +/// list to process template -static void -createBodyOfOp(Op &op, Fortran::lower::AbstractConverter &converter, - mlir::Location &loc, Fortran::lower::pft::Evaluation &eval, - const Fortran::parser::OmpClauseList *clauses = nullptr, - const SmallVector &args = {}, - bool outerCombined = false) { +static void createBodyOfOp( + Op &op, Fortran::lower::AbstractConverter &converter, mlir::Location &loc, + Fortran::lower::pft::Evaluation &eval, + const Fortran::parser::OmpClauseList *clauses = nullptr, + const SmallVector &args = {}, + bool outerCombined = false, + [[maybe_unused]] const Fortran::parser::OmpClauseList *otherClauses = + nullptr) { fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); // If an argument for the region is provided then create the block with that // argument. Also update the symbol's address with the mlir argument value. @@ -531,12 +581,13 @@ // Handle privatization. Do not privatize if this is the outer operation. if (clauses && !outerCombined) { - bool lastPrivateOp = privatizeVars(op, converter, *clauses, eval); + bool lastPrivateOp = + privatizeVars(op, converter, *clauses, eval, otherClauses); // LastPrivatization, due to introduction of // new control flow, changes the insertion point, // thus restore it. // TODO: Clean up later a bit to avoid this many sets and resets. - if (lastPrivateOp) + if (lastPrivateOp && !std::is_same_v) resetBeforeTerminator(firOpBuilder, storeOp, block); } @@ -1375,12 +1426,18 @@ std::get( sectionsConstruct->t) .t); + const Fortran::parser::OmpClauseList §ionsEndClauseList = + std::get( + std::get( + sectionsConstruct->t) + .t); // Currently only private/firstprivate clause is handled, and // all privatization is done within `omp.section` operations. mlir::omp::SectionOp sectionOp = firOpBuilder.create(currentLocation); - createBodyOfOp(sectionOp, converter, currentLocation, eval, - §ionsClauseList); + createBodyOfOp( + sectionOp, converter, currentLocation, eval, §ionsClauseList, + /*iv=*/{}, /*outerCombined=*/false, §ionsEndClauseList); } static void diff --git a/flang/test/Lower/OpenMP/sections.f90 b/flang/test/Lower/OpenMP/sections.f90 --- a/flang/test/Lower/OpenMP/sections.f90 +++ b/flang/test/Lower/OpenMP/sections.f90 @@ -108,3 +108,113 @@ alpha = alpha * 5 !$omp end sections end subroutine + +subroutine lastprivate() + integer :: x +!CHECK: %[[X:.*]] = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFlastprivateEx"} +!CHECK: omp.sections { + !$omp sections lastprivate(x) +!CHECK: omp.section { +!CHECK: %[[PRIVATE_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFlastprivateEx"} +!CHECK: %[[const:.*]] = arith.constant 10 : i32 +!CHECK: %[[temp:.*]] = fir.load %1 : !fir.ref +!CHECK: %[[result:.*]] = arith.muli %c10_i32, %[[temp]] : i32 +!CHECK: fir.store %[[result]] to %[[PRIVATE_X]] : !fir.ref +!CHECK: omp.terminator +!CHECK: } + !$omp section + x = x * 10 +!CHECK: omp.section { +!CHECK: %[[PRIVATE_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFlastprivateEx"} +!CHECK: %[[true:.*]] = arith.constant true +!CHECK: %[[temp:.*]] = fir.load %1 : !fir.ref +!CHECK: %[[const:.*]] = arith.constant 1 : i32 +!CHECK: %[[result:.*]] = arith.addi %[[temp]], %[[const]] : i32 +!CHECK: fir.store %[[result]] to %[[PRIVATE_X]] : !fir.ref +!CHECK: scf.if %[[true]] { +!CHECK: %[[temp:.*]] = fir.load %1 : !fir.ref +!CHECK: fir.store %[[temp]] to %[[X]] : !fir.ref +!CHECK: } +!CHECK: omp.terminator +!CHECK: } + !$omp section + x = x + 1 +!CHECK: omp.terminator +!CHECK: } + !$omp end sections + +!CHECK: omp.sections { + !$omp sections firstprivate(x) lastprivate(x) +!CHECK: omp.section { +!CHECK: %[[PRIVATE_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFlastprivateEx"} +!CHECK: %[[temp:.*]] = fir.load %[[X]] : !fir.ref +!CHECK: fir.store %[[temp]] to %[[PRIVATE_X]] : !fir.ref +!CHECK: omp.barrier +!CHECK: %[[const:.*]] = arith.constant 10 : i32 +!CHECK: %[[temp:.*]] = fir.load %1 : !fir.ref +!CHECK: %[[result:.*]] = arith.muli %c10_i32, %[[temp]] : i32 +!CHECK: fir.store %[[result]] to %[[PRIVATE_X]] : !fir.ref +!CHECK: omp.terminator +!CHECK: } + !$omp section + x = x * 10 +!CHECK: omp.section { +!CHECK: %[[PRIVATE_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFlastprivateEx"} +!CHECK: %[[temp:.*]] = fir.load %[[X]] : !fir.ref +!CHECK: fir.store %[[temp]] to %[[PRIVATE_X]] : !fir.ref +!CHECK: omp.barrier +!CHECK: %[[true:.*]] = arith.constant true +!CHECK: %[[temp:.*]] = fir.load %[[PRIVATE_X]] : !fir.ref +!CHECK: %[[const:.*]] = arith.constant 1 : i32 +!CHECK: %[[result:.*]] = arith.addi %[[temp]], %[[const]] : i32 +!CHECK: fir.store %[[result]] to %[[PRIVATE_X]] : !fir.ref +!CHECK: scf.if %true { +!CHECK: %[[temp:.*]] = fir.load %1 : !fir.ref +!CHECK: fir.store %[[temp]] to %[[X]] : !fir.ref +!CHECK: } +!CHECK: omp.terminator +!CHECK: } + !$omp section + x = x + 1 +!CHECK: omp.terminator +!CHECK: } + !$omp end sections + +!CHECK: omp.sections nowait { + !$omp sections firstprivate(x) lastprivate(x) +!CHECK: omp.section { +!CHECK: %[[PRIVATE_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFlastprivateEx"} +!CHECK: %[[temp:.*]] = fir.load %[[X]] : !fir.ref +!CHECK: fir.store %[[temp]] to %[[PRIVATE_X]] : !fir.ref +!CHECK: omp.barrier +!CHECK: %[[const:.*]] = arith.constant 10 : i32 +!CHECK: %[[temp:.*]] = fir.load %1 : !fir.ref +!CHECK: %[[result:.*]] = arith.muli %c10_i32, %[[temp]] : i32 +!CHECK: fir.store %[[result]] to %[[PRIVATE_X]] : !fir.ref +!CHECK: omp.terminator +!CHECK: } + !$omp section + x = x * 10 +!CHECK: omp.section { +!CHECK: %[[PRIVATE_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFlastprivateEx"} +!CHECK: %[[temp:.*]] = fir.load %[[X]] : !fir.ref +!CHECK: fir.store %[[temp]] to %[[PRIVATE_X]] : !fir.ref +!CHECK: omp.barrier +!CHECK: %[[true:.*]] = arith.constant true +!CHECK: %[[temp:.*]] = fir.load %[[PRIVATE_X]] : !fir.ref +!CHECK: %[[const:.*]] = arith.constant 1 : i32 +!CHECK: %[[result:.*]] = arith.addi %[[temp]], %[[const]] : i32 +!CHECK: fir.store %[[result]] to %[[PRIVATE_X]] : !fir.ref +!CHECK: scf.if %true { +!CHECK: %[[temp:.*]] = fir.load %1 : !fir.ref +!CHECK: fir.store %[[temp]] to %[[X]] : !fir.ref +!CHECK: omp.barrier +!CHECK: } +!CHECK: omp.terminator +!CHECK: } + !$omp section + x = x + 1 +!CHECK: omp.terminator +!CHECK: } + !$omp end sections nowait +end subroutine