llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-flang-fir-hlfir Author: Tom Eccles (tblah) <details> <summary>Changes</summary> Based on top of #<!-- -->144013 I was really hoping this would also work for `hostEvalInfo` but unfortunately that needed to be shared to a greater degree. The same technique should work for that but it would need that class to be made public and then the state kept between calls to `genOpenMP*Construct`, which felt like more trouble than it was worth. I'm open to abandoning this patch if solving one global variable doesn't feel worth this much churn. Making these changes I was wondering if we should implement this file with one big class to wrap up all the state passed to every function. Any thoughts? --- Patch is 75.76 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/144087.diff 1 Files Affected: - (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+310-250) ``````````diff diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index 060eba1b906e3..9c0bfa95f8382 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -48,6 +48,10 @@ using namespace Fortran::common::openmp; static llvm::cl::opt<bool> DumpAtomicAnalysis("fdebug-dump-atomic-analysis"); +namespace { +struct OmpLoweringContext; +} // namespace + //===----------------------------------------------------------------------===// // Code generation helper functions //===----------------------------------------------------------------------===// @@ -55,6 +59,7 @@ static llvm::cl::opt<bool> DumpAtomicAnalysis("fdebug-dump-atomic-analysis"); static void genOMPDispatch(lower::AbstractConverter &converter, lower::SymMap &symTable, semantics::SemanticsContext &semaCtx, + OmpLoweringContext &ompCtx, lower::pft::Evaluation &eval, mlir::Location loc, const ConstructQueue &queue, ConstructQueue::const_iterator item); @@ -191,18 +196,28 @@ class HostEvalInfo { llvm::SmallVector<const semantics::Symbol *> iv; bool loopNestApplied = false, parallelApplied = false; }; -} // namespace /// Stack of \see HostEvalInfo to represent the current nest of \c omp.target /// operations being created. /// /// The current implementation prevents nested 'target' regions from breaking /// the handling of the outer region by keeping a stack of information -/// structures, but it will probably still require some further work to support -/// reverse offloading. -static llvm::SmallVector<HostEvalInfo, 0> hostEvalInfo; -static llvm::SmallVector<const parser::OpenMPSectionsConstruct *, 0> - sectionsStack; +/// structures, but it will probably still require some further work to +/// support reverse offloading. +/// +/// This has to be a global rather than in OmpLoweringContext because different +/// calls to void Fortran::lower::genOpenMPConstruct and +/// Fortran::lower::genOpenMPDeclarativeConstruct need to share the same +/// instance. FIXME: Maybe this should be promoted into the interface for those +/// functions. +llvm::SmallVector<HostEvalInfo, 0> hostEvalInfo; + +struct OmpLoweringContext { + /// Stack of parse tree information about the sections construct to allow each + /// section to be lowered as part of the enclosing sections construct. + llvm::SmallVector<const parser::OpenMPSectionsConstruct *, 0> sectionsStack; +}; +} // namespace /// Bind symbols to their corresponding entry block arguments. /// @@ -1151,10 +1166,11 @@ struct OpWithBodyGenInfo { OpWithBodyGenInfo(lower::AbstractConverter &converter, lower::SymMap &symTable, - semantics::SemanticsContext &semaCtx, mlir::Location loc, + semantics::SemanticsContext &semaCtx, + OmpLoweringContext &ompCtx, mlir::Location loc, lower::pft::Evaluation &eval, llvm::omp::Directive dir) - : converter(converter), symTable(symTable), semaCtx(semaCtx), loc(loc), - eval(eval), dir(dir) {} + : converter(converter), symTable(symTable), semaCtx(semaCtx), + ompCtx(ompCtx), loc(loc), eval(eval), dir(dir) {} OpWithBodyGenInfo &setClauses(const List<Clause> *value) { clauses = value; @@ -1187,6 +1203,8 @@ struct OpWithBodyGenInfo { lower::SymMap &symTable; /// [in] Semantics context semantics::SemanticsContext &semaCtx; + /// [in] OpenMP context + OmpLoweringContext &ompCtx; /// [in] location in source code. mlir::Location loc; /// [in] current PFT node/evaluation. @@ -1290,8 +1308,8 @@ static void createBodyOfOp(mlir::Operation &op, const OpWithBodyGenInfo &info, if (!info.genSkeletonOnly) { if (ConstructQueue::const_iterator next = std::next(item); next != queue.end()) { - genOMPDispatch(info.converter, info.symTable, info.semaCtx, info.eval, - info.loc, queue, next); + genOMPDispatch(info.converter, info.symTable, info.semaCtx, info.ompCtx, + info.eval, info.loc, queue, next); } else { // genFIR(Evaluation&) tries to patch up unterminated blocks, causing // a lot of complications for our approach if the terminator generation @@ -1383,10 +1401,10 @@ static void createBodyOfOp(mlir::Operation &op, const OpWithBodyGenInfo &info, static void genBodyOfTargetDataOp( lower::AbstractConverter &converter, lower::SymMap &symTable, - semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval, - mlir::omp::TargetDataOp &dataOp, const EntryBlockArgs &args, - const mlir::Location ¤tLocation, const ConstructQueue &queue, - ConstructQueue::const_iterator item) { + semantics::SemanticsContext &semaCtx, OmpLoweringContext &ompCtx, + lower::pft::Evaluation &eval, mlir::omp::TargetDataOp &dataOp, + const EntryBlockArgs &args, const mlir::Location ¤tLocation, + const ConstructQueue &queue, ConstructQueue::const_iterator item) { fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); genEntryBlock(firOpBuilder, args, dataOp.getRegion()); @@ -1414,8 +1432,8 @@ static void genBodyOfTargetDataOp( if (ConstructQueue::const_iterator next = std::next(item); next != queue.end()) { - genOMPDispatch(converter, symTable, semaCtx, eval, currentLocation, queue, - next); + genOMPDispatch(converter, symTable, semaCtx, ompCtx, eval, currentLocation, + queue, next); } else { genNestedEvaluations(converter, eval); } @@ -1458,10 +1476,11 @@ static void genIntermediateCommonBlockAccessors( // all the symbols present in mapSymbols as block arguments to this block. static void genBodyOfTargetOp( lower::AbstractConverter &converter, lower::SymMap &symTable, - semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval, - mlir::omp::TargetOp &targetOp, const EntryBlockArgs &args, - const mlir::Location ¤tLocation, const ConstructQueue &queue, - ConstructQueue::const_iterator item, DataSharingProcessor &dsp) { + semantics::SemanticsContext &semaCtx, OmpLoweringContext &ompCtx, + lower::pft::Evaluation &eval, mlir::omp::TargetOp &targetOp, + const EntryBlockArgs &args, const mlir::Location ¤tLocation, + const ConstructQueue &queue, ConstructQueue::const_iterator item, + DataSharingProcessor &dsp) { fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); auto argIface = llvm::cast<mlir::omp::BlockArgOpenMPOpInterface>(*targetOp); @@ -1606,8 +1625,8 @@ static void genBodyOfTargetOp( if (ConstructQueue::const_iterator next = std::next(item); next != queue.end()) { - genOMPDispatch(converter, symTable, semaCtx, eval, currentLocation, queue, - next); + genOMPDispatch(converter, symTable, semaCtx, ompCtx, eval, currentLocation, + queue, next); } else { genNestedEvaluations(converter, eval); } @@ -1703,8 +1722,9 @@ static void genFlushClauses(lower::AbstractConverter &converter, static void genLoopNestClauses(lower::AbstractConverter &converter, semantics::SemanticsContext &semaCtx, - lower::pft::Evaluation &eval, const List<Clause> &clauses, - mlir::Location loc, mlir::omp::LoopNestOperands &clauseOps, + OmpLoweringContext &ompCtx, lower::pft::Evaluation &eval, + const List<Clause> &clauses, mlir::Location loc, + mlir::omp::LoopNestOperands &clauseOps, llvm::SmallVectorImpl<const semantics::Symbol *> &iv) { ClauseProcessor cp(converter, semaCtx, clauses); @@ -1746,8 +1766,9 @@ genOrderedRegionClauses(lower::AbstractConverter &converter, static void genParallelClauses( lower::AbstractConverter &converter, semantics::SemanticsContext &semaCtx, - lower::StatementContext &stmtCtx, const List<Clause> &clauses, - mlir::Location loc, mlir::omp::ParallelOperands &clauseOps, + lower::StatementContext &stmtCtx, OmpLoweringContext &ompCtx, + const List<Clause> &clauses, mlir::Location loc, + mlir::omp::ParallelOperands &clauseOps, llvm::SmallVectorImpl<const semantics::Symbol *> &reductionSyms) { ClauseProcessor cp(converter, semaCtx, clauses); cp.processAllocate(clauseOps); @@ -1812,9 +1833,9 @@ static void genSingleClauses(lower::AbstractConverter &converter, static void genTargetClauses( lower::AbstractConverter &converter, semantics::SemanticsContext &semaCtx, lower::SymMap &symTable, lower::StatementContext &stmtCtx, - lower::pft::Evaluation &eval, const List<Clause> &clauses, - mlir::Location loc, mlir::omp::TargetOperands &clauseOps, - DefaultMapsTy &defaultMaps, + OmpLoweringContext &ompCtx, lower::pft::Evaluation &eval, + const List<Clause> &clauses, mlir::Location loc, + mlir::omp::TargetOperands &clauseOps, DefaultMapsTy &defaultMaps, llvm::SmallVectorImpl<const semantics::Symbol *> &hasDeviceAddrSyms, llvm::SmallVectorImpl<const semantics::Symbol *> &isDevicePtrSyms, llvm::SmallVectorImpl<const semantics::Symbol *> &mapSyms) { @@ -1956,8 +1977,9 @@ static void genWorkshareClauses(lower::AbstractConverter &converter, static void genTeamsClauses( lower::AbstractConverter &converter, semantics::SemanticsContext &semaCtx, - lower::StatementContext &stmtCtx, const List<Clause> &clauses, - mlir::Location loc, mlir::omp::TeamsOperands &clauseOps, + lower::StatementContext &stmtCtx, OmpLoweringContext &ompCtx, + const List<Clause> &clauses, mlir::Location loc, + mlir::omp::TeamsOperands &clauseOps, llvm::SmallVectorImpl<const semantics::Symbol *> &reductionSyms) { ClauseProcessor cp(converter, semaCtx, clauses); cp.processAllocate(clauseOps); @@ -2027,7 +2049,7 @@ static mlir::omp::CancellationPointOp genCancellationPointOp( static mlir::omp::CriticalOp genCriticalOp(lower::AbstractConverter &converter, lower::SymMap &symTable, - semantics::SemanticsContext &semaCtx, + semantics::SemanticsContext &semaCtx, OmpLoweringContext &ompCtx, lower::pft::Evaluation &eval, mlir::Location loc, const ConstructQueue &queue, ConstructQueue::const_iterator item, const std::optional<parser::Name> &name) { @@ -2051,7 +2073,7 @@ genCriticalOp(lower::AbstractConverter &converter, lower::SymMap &symTable, } return genOpWithBody<mlir::omp::CriticalOp>( - OpWithBodyGenInfo(converter, symTable, semaCtx, loc, eval, + OpWithBodyGenInfo(converter, symTable, semaCtx, ompCtx, loc, eval, llvm::omp::Directive::OMPD_critical), queue, item, nameAttr); } @@ -2071,9 +2093,10 @@ genFlushOp(lower::AbstractConverter &converter, lower::SymMap &symTable, static mlir::omp::LoopNestOp genLoopNestOp( lower::AbstractConverter &converter, lower::SymMap &symTable, - semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval, - mlir::Location loc, const ConstructQueue &queue, - ConstructQueue::const_iterator item, mlir::omp::LoopNestOperands &clauseOps, + semantics::SemanticsContext &semaCtx, OmpLoweringContext &ompCtx, + lower::pft::Evaluation &eval, mlir::Location loc, + const ConstructQueue &queue, ConstructQueue::const_iterator item, + mlir::omp::LoopNestOperands &clauseOps, llvm::ArrayRef<const semantics::Symbol *> iv, llvm::ArrayRef< std::pair<mlir::omp::BlockArgOpenMPOpInterface, const EntryBlockArgs &>> @@ -2088,7 +2111,7 @@ static mlir::omp::LoopNestOp genLoopNestOp( getCollapsedLoopEval(eval, getCollapseValue(item->clauses)); return genOpWithBody<mlir::omp::LoopNestOp>( - OpWithBodyGenInfo(converter, symTable, semaCtx, loc, *nestedEval, + OpWithBodyGenInfo(converter, symTable, semaCtx, ompCtx, loc, *nestedEval, directive) .setClauses(&item->clauses) .setDataSharingProcessor(&dsp) @@ -2098,9 +2121,9 @@ static mlir::omp::LoopNestOp genLoopNestOp( static mlir::omp::LoopOp genLoopOp(lower::AbstractConverter &converter, lower::SymMap &symTable, - semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval, - mlir::Location loc, const ConstructQueue &queue, - ConstructQueue::const_iterator item) { + semantics::SemanticsContext &semaCtx, OmpLoweringContext &ompCtx, + lower::pft::Evaluation &eval, mlir::Location loc, + const ConstructQueue &queue, ConstructQueue::const_iterator item) { mlir::omp::LoopOperands loopClauseOps; llvm::SmallVector<const semantics::Symbol *> loopReductionSyms; genLoopClauses(converter, semaCtx, item->clauses, loc, loopClauseOps, @@ -2113,7 +2136,7 @@ genLoopOp(lower::AbstractConverter &converter, lower::SymMap &symTable, mlir::omp::LoopNestOperands loopNestClauseOps; llvm::SmallVector<const semantics::Symbol *> iv; - genLoopNestClauses(converter, semaCtx, eval, item->clauses, loc, + genLoopNestClauses(converter, semaCtx, ompCtx, eval, item->clauses, loc, loopNestClauseOps, iv); EntryBlockArgs loopArgs; @@ -2124,7 +2147,7 @@ genLoopOp(lower::AbstractConverter &converter, lower::SymMap &symTable, auto loopOp = genWrapperOp<mlir::omp::LoopOp>(converter, loc, loopClauseOps, loopArgs); - genLoopNestOp(converter, symTable, semaCtx, eval, loc, queue, item, + genLoopNestOp(converter, symTable, semaCtx, ompCtx, eval, loc, queue, item, loopNestClauseOps, iv, {{loopOp, loopArgs}}, llvm::omp::Directive::OMPD_loop, dsp); return loopOp; @@ -2133,25 +2156,25 @@ genLoopOp(lower::AbstractConverter &converter, lower::SymMap &symTable, static mlir::omp::MaskedOp genMaskedOp(lower::AbstractConverter &converter, lower::SymMap &symTable, lower::StatementContext &stmtCtx, - semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval, - mlir::Location loc, const ConstructQueue &queue, - ConstructQueue::const_iterator item) { + semantics::SemanticsContext &semaCtx, OmpLoweringContext &ompCtx, + lower::pft::Evaluation &eval, mlir::Location loc, + const ConstructQueue &queue, ConstructQueue::const_iterator item) { mlir::omp::MaskedOperands clauseOps; genMaskedClauses(converter, semaCtx, stmtCtx, item->clauses, loc, clauseOps); return genOpWithBody<mlir::omp::MaskedOp>( - OpWithBodyGenInfo(converter, symTable, semaCtx, loc, eval, + OpWithBodyGenInfo(converter, symTable, semaCtx, ompCtx, loc, eval, llvm::omp::Directive::OMPD_masked), queue, item, clauseOps); } static mlir::omp::MasterOp genMasterOp(lower::AbstractConverter &converter, lower::SymMap &symTable, - semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval, - mlir::Location loc, const ConstructQueue &queue, - ConstructQueue::const_iterator item) { + semantics::SemanticsContext &semaCtx, OmpLoweringContext &ompCtx, + lower::pft::Evaluation &eval, mlir::Location loc, + const ConstructQueue &queue, ConstructQueue::const_iterator item) { return genOpWithBody<mlir::omp::MasterOp>( - OpWithBodyGenInfo(converter, symTable, semaCtx, loc, eval, + OpWithBodyGenInfo(converter, symTable, semaCtx, ompCtx, loc, eval, llvm::omp::Directive::OMPD_master), queue, item); } @@ -2168,21 +2191,21 @@ genOrderedOp(lower::AbstractConverter &converter, lower::SymMap &symTable, static mlir::omp::OrderedRegionOp genOrderedRegionOp(lower::AbstractConverter &converter, lower::SymMap &symTable, semantics::SemanticsContext &semaCtx, - lower::pft::Evaluation &eval, mlir::Location loc, - const ConstructQueue &queue, + OmpLoweringContext &ompCtx, lower::pft::Evaluation &eval, + mlir::Location loc, const ConstructQueue &queue, ConstructQueue::const_iterator item) { mlir::omp::OrderedRegionOperands clauseOps; genOrderedRegionClauses(converter, semaCtx, item->clauses, loc, clauseOps); return genOpWithBody<mlir::omp::OrderedRegionOp>( - OpWithBodyGenInfo(converter, symTable, semaCtx, loc, eval, + OpWithBodyGenInfo(converter, symTable, semaCtx, ompCtx, loc, eval, llvm::omp::Directive::OMPD_ordered), queue, item, clauseOps); } static mlir::omp::ParallelOp genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable, - semantics::SemanticsContext &semaCtx, + semantics::SemanticsContext &semaCtx, OmpLoweringContext &ompCtx, lower::pft::Evaluation &eval, mlir::Location loc, const ConstructQueue &queue, ConstructQueue::const_iterator item, mlir::omp::ParallelOperands &clauseOps, @@ -2192,7 +2215,7 @@ genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable, "expected valid DataSharingProcessor"); OpWithBodyGenInfo genInfo = - OpWithBodyGenInfo(converter, symTable, semaCtx, loc, eval, + OpWithBodyGenInfo(converter, symTable, semaCtx, ompCtx, loc, eval, llvm::omp::Directive::OMPD_parallel) .setClauses(&item->clauses) .setEntryBlockArgs(&args) @@ -2220,14 +2243,14 @@ genScanOp(lower::AbstractConverter &converter, lower::SymMap &symTable, /// lowered here with correct reduction symbol remapping. static mlir::omp::SectionsOp genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable, - semantics::SemanticsContext &semaCtx, + semantics::SemanticsContext &semaCtx, OmpLoweringContext &ompCtx, lower::pft::Evaluation &eval, mlir::Location loc, const ConstructQueue &queue, ConstructQueue::const_iterator item) { - assert(!sectionsStack.empty()); + assert(!ompCtx.sectionsStack.empty()); const auto §ionBlocks = - std::get<parser::OmpSectionBlocks>(sectionsStack.back()->t); - sectionsStack.pop_back(); + std::get<parser::OmpSectionBlocks>(ompCtx.sectionsStack.back()->t); + ompCtx.sectionsStack.pop_back(); mlir::omp::SectionsOperands clauseOps; llvm::SmallVector<const semantics::Symbol *> reductionSyms; genSectionsClauses(converter, semaCtx, item->clauses, loc, clauseOps, @@ -2295,7 +2318,7 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable, builder.setInsertionPoint(terminator); genOpWithBody<mlir::omp::SectionOp>( - OpWithBodyGenInfo(converter, symTable, semaCtx, loc, nestedEval, + OpWithBodyGenInfo(converter, symTable, semaCtx, ompCtx, loc, nestedEval, llvm::omp::Directive::OMPD_section) .setClauses(§ionQueue.begin()->clauses) .setDataSharingProcessor(&dsp) @@ -2355,14 +2378,14 @@ genScopeOp(lower::AbstractConverter &converter, lower::SymMap &symTable, static mlir::omp::SingleOp genSingleOp(lower::AbstractConverter &converter, lower::SymMap &symTable, - semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval, - mlir::Location loc, const ConstructQueue &queue, - ConstructQueue::const_iterator item) { + semantics::SemanticsContext &semaCtx, OmpLoweringContext &ompCtx, + lower::pft::Evaluation &eval, mlir::Location loc, + const ConstructQueue &queue, ConstructQueue::const_iterator item) { mlir::omp::SingleOperands clauseOps; genSingleClauses(converter, semaCtx, item->clauses, loc, clauseOps); return genOpWithBody<mlir::omp::SingleOp>( - OpWithBodyGenInfo(converter, symTable, semaCtx, loc, eval, + OpWithBodyGenInfo(converter, symTable, semaCtx, ompCtx, loc, eval, llvm::omp::Directive::OMPD_single) .setClauses(&item->clauses), queue, item, clauseOps); @@ -2371,9 +2394,9 @@ genSingleOp(lower::AbstractConverter &converter, lower::SymMap &symTable, static mlir::omp::TargetOp genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable, lower::StatementContext &stmtCtx, - semantic... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/144087 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
