diff --git a/flang/include/flang/Lower/AbstractConverter.h b/flang/include/flang/Lower/AbstractConverter.h --- a/flang/include/flang/Lower/AbstractConverter.h +++ b/flang/include/flang/Lower/AbstractConverter.h @@ -103,8 +103,10 @@ virtual bool createHostAssociateVarClone(const Fortran::semantics::Symbol &sym) = 0; - virtual void copyHostAssociateVar(const Fortran::semantics::Symbol &sym, - mlir::Block *lastPrivBlock = nullptr) = 0; + virtual void + copyHostAssociateVar(const Fortran::semantics::Symbol &sym, + mlir::Block *lastPrivBlock = nullptr, + mlir::Operation *targetOperation = nullptr) = 0; /// Collect the set of symbols with \p flag in \p eval /// region if \p collectSymbols is true. Likewise, collect the diff --git a/flang/include/flang/Lower/PFTBuilder.h b/flang/include/flang/Lower/PFTBuilder.h --- a/flang/include/flang/Lower/PFTBuilder.h +++ b/flang/include/flang/Lower/PFTBuilder.h @@ -271,6 +271,7 @@ }}); } + bool operator==(const Evaluation &that) const { return this == &that; } LLVM_DUMP_METHOD void dump() const; /// Return the first non-nop successor of an evaluation, possibly exiting diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp --- a/flang/lib/Lower/Bridge.cpp +++ b/flang/lib/Lower/Bridge.cpp @@ -517,9 +517,10 @@ } // FIXME: Generalize this function, so that lastPrivBlock can be removed - void - copyHostAssociateVar(const Fortran::semantics::Symbol &sym, - mlir::Block *lastPrivBlock = nullptr) override final { + void copyHostAssociateVar( + const Fortran::semantics::Symbol &sym, + mlir::Block *lastPrivBlock = nullptr, + mlir::Operation *targetOperation = nullptr) override final { // 1) Fetch the original copy of the variable. assert(sym.has() && "No host-association found"); @@ -537,13 +538,15 @@ // 3) Perform the assignment. mlir::OpBuilder::InsertPoint insPt = builder->saveInsertionPoint(); - if (lastPrivBlock) + if (targetOperation) + builder->setInsertionPoint(targetOperation); + else if (lastPrivBlock) builder->setInsertionPointToStart(lastPrivBlock); else builder->setInsertionPointAfter(fir::getBase(exv).getDefiningOp()); fir::ExtendedValue lhs, rhs; - if (lastPrivBlock) { + if (lastPrivBlock || targetOperation) { // lastprivate case lhs = hexv; rhs = exv; 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 @@ -78,6 +78,39 @@ converter.copyHostAssociateVar(*sym, lastPrivBlock); } +static void +privatizeSymbolInSectionOp(Fortran::lower::AbstractConverter &converter, + const Fortran::semantics::Symbol *sym, + Fortran::lower::pft::Evaluation &eval, + omp::SectionOp *sectionOp) { + // Privatization for symbols which are pre-determined (like loop index + // variables) happen separately, for everything else privatize here. + if (sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined)) + return; + fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); + bool success = converter.createHostAssociateVarClone(*sym); + (void)success; + assert(success && "Privatization failed due to existing binding"); + if (sym->test(Fortran::semantics::Symbol::Flag::OmpFirstPrivate)) + converter.copyHostAssociateVar(*sym); + if (sym->test(Fortran::semantics::Symbol::Flag::OmpLastPrivate) && + eval == eval.parentConstruct->getLastNestedEvaluation()) { + // `lastprivate` symbols are added only to the last + // `omp.section` operation in a `omp.sections` operation. + // A `lastprivate` variable's privatization shall be split into two parts: + // omp.section { + // fir.allocate + // + // fir.store to original variable + // } + auto sectionOpOperationsIter = sectionOp->region().back().rbegin(); + auto &targetOper = *(++sectionOpOperationsIter); + converter.copyHostAssociateVar(*sym, nullptr, + sectionOp->region().back().getTerminator()); + firOpBuilder.setInsertionPointAfter(&targetOper); + } +} + template static bool privatizeVars(Op &op, Fortran::lower::AbstractConverter &converter, const Fortran::parser::OmpClauseList &opClauseList, @@ -109,7 +142,7 @@ } else if (const auto &lastPrivateClause = std::get_if( &clause.u)) { - // TODO: Add lastprivate support for sections construct, simd construct + // TODO: Add lastprivate support for simd construct if (std::is_same_v) { omp::WsLoopOp *wsLoopOp = dyn_cast(&op); mlir::Operation *lastOper = wsLoopOp->region().back().getTerminator(); @@ -155,10 +188,12 @@ mlir::scf::IfOp ifOp = firOpBuilder.create( wsLoopOp->getLoc(), cmpOp, /*else*/ false); lastPrivBlock = &ifOp.getThenRegion().front(); - } else { + } + // `lastprivate` variables in `omp.section` operation + // are handled elsewhere + else if (!std::is_same_v) TODO(converter.getCurrentLocation(), "lastprivate clause in constructs other than worksharing-loop"); - } collectOmpObjectListSymbol(lastPrivateClause->v, privatizedSymbols); hasLastPrivateOp = true; } @@ -206,7 +241,11 @@ else firOpBuilder.setInsertionPointToStart(firOpBuilder.getAllocaBlock()); for (auto sym : privatizedSymbols) { - privatizeSymbol(converter, sym, lastPrivBlock); + if (std::is_same_v) + privatizeSymbolInSectionOp(converter, sym, eval, + dyn_cast(&op)); + else + privatizeSymbol(converter, sym, lastPrivBlock); if (sym->test(Fortran::semantics::Symbol::Flag::OmpFirstPrivate) && sym->test(Fortran::semantics::Symbol::Flag::OmpLastPrivate)) needBarrier = true; @@ -227,8 +266,11 @@ // Emit implicit barrier for linear clause. Maybe on somewhere else. if (needBarrier) firOpBuilder.create(converter.getCurrentLocation()); - - firOpBuilder.restoreInsertionPoint(insPt); + // For a `omp.section` operation with `lastprivate`, do not reset the + // insertion point as the privatization handling has already set it to + // appropriate location + if (!(hasLastPrivateOp && std::is_same_v)) + firOpBuilder.restoreInsertionPoint(insPt); return hasLastPrivateOp; } @@ -523,7 +565,7 @@ // 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); } 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,69 @@ 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, %2 : 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 %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: %[[temp:.*]] = fir.load %1 : !fir.ref +!CHECK: fir.store %[[temp]] to %[[X]] : !fir.ref +!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, %3 : 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: %[[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: %[[temp:.*]] = fir.load %1 : !fir.ref +!CHECK: fir.store %[[temp]] to %[[X]] : !fir.ref +!CHECK: omp.terminator +!CHECK: } + !$omp section + x = x + 1 +!CHECK: omp.terminator +!CHECK: } + !$omp end sections +end subroutine