[flang-commits] [compiler-rt] [clang] [flang] [clang-tools-extra] [llvm] [Flang][OpenMP] Restructure recursive lowering in `createBodyOfOp` (PR #77761)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Fri Jan 19 08:59:07 PST 2024
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/77761
>From 62f31654ec66fe0e2a27200d0484d3c70d4ce2c1 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 20 Dec 2023 15:12:04 -0600
Subject: [PATCH 01/11] [Flang][OpenMP] Separate creation of work-sharing and
SIMD loops, NFC
These two constructs were both handled in `genOMP` for loop constructs.
There is some shared code between the two, but there are also enough
differences to separate these two cases into individual functions.
The shared code may be placed into a helper function later if needed.
Recursive lowering [1/5]
---
flang/lib/Lower/OpenMP.cpp | 252 ++++++++++++++++++++++---------------
1 file changed, 153 insertions(+), 99 deletions(-)
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index c3a570bf15ea0d..350cb29121da93 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -2968,24 +2968,150 @@ genOMP(Fortran::lower::AbstractConverter &converter,
standaloneConstruct.u);
}
-static void genOMP(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
- Fortran::semantics::SemanticsContext &semanticsContext,
- const Fortran::parser::OpenMPLoopConstruct &loopConstruct) {
+static void
+createSimdLoop(Fortran::lower::AbstractConverter &converter,
+ Fortran::lower::pft::Evaluation &eval,
+ llvm::omp::Directive ompDirective,
+ const Fortran::parser::OmpClauseList &loopOpClauseList,
+ mlir::Location loc) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- llvm::SmallVector<mlir::Value> lowerBound, upperBound, step, linearVars,
- linearStepVars, reductionVars;
+ DataSharingProcessor dsp(converter, loopOpClauseList, eval);
+ dsp.processStep1();
+
+ Fortran::lower::StatementContext stmtCtx;
mlir::Value scheduleChunkClauseOperand;
- mlir::IntegerAttr orderedClauseOperand;
+ llvm::SmallVector<mlir::Value> lowerBound, upperBound, step, reductionVars;
+ llvm::SmallVector<const Fortran::semantics::Symbol *> iv;
+ llvm::SmallVector<mlir::Attribute> reductionDeclSymbols;
+ mlir::omp::ClauseOrderKindAttr orderClauseOperand;
+ std::size_t loopVarTypeSize;
+
+ ClauseProcessor cp(converter, loopOpClauseList);
+ cp.processCollapse(loc, eval, lowerBound, upperBound, step, iv,
+ loopVarTypeSize);
+ cp.processScheduleChunk(stmtCtx, scheduleChunkClauseOperand);
+ cp.processReduction(loc, reductionVars, reductionDeclSymbols);
+ cp.processTODO<Fortran::parser::OmpClause::Linear,
+ Fortran::parser::OmpClause::Order>(loc, ompDirective);
+
+ // The types of lower bound, upper bound, and step are converted into the
+ // type of the loop variable if necessary.
+ mlir::Type loopVarType = getLoopVarType(converter, loopVarTypeSize);
+ for (unsigned it = 0; it < (unsigned)lowerBound.size(); it++) {
+ lowerBound[it] =
+ firOpBuilder.createConvert(loc, loopVarType, lowerBound[it]);
+ upperBound[it] =
+ firOpBuilder.createConvert(loc, loopVarType, upperBound[it]);
+ step[it] = firOpBuilder.createConvert(loc, loopVarType, step[it]);
+ }
+
+ llvm::SmallVector<mlir::Value> alignedVars, nontemporalVars;
+ mlir::Value ifClauseOperand;
+ mlir::IntegerAttr simdlenClauseOperand, safelenClauseOperand;
+ cp.processIf(Fortran::parser::OmpIfClause::DirectiveNameModifier::Simd,
+ ifClauseOperand);
+ cp.processSimdlen(simdlenClauseOperand);
+ cp.processSafelen(safelenClauseOperand);
+ cp.processTODO<Fortran::parser::OmpClause::Aligned,
+ Fortran::parser::OmpClause::Allocate,
+ Fortran::parser::OmpClause::Nontemporal>(loc, ompDirective);
+
+ mlir::TypeRange resultType;
+ auto simdLoopOp = firOpBuilder.create<mlir::omp::SimdLoopOp>(
+ loc, resultType, lowerBound, upperBound, step, alignedVars,
+ /*alignment_values=*/nullptr, ifClauseOperand, nontemporalVars,
+ orderClauseOperand, simdlenClauseOperand, safelenClauseOperand,
+ /*inclusive=*/firOpBuilder.getUnitAttr());
+ createBodyOfOp<mlir::omp::SimdLoopOp>(simdLoopOp, converter, loc, eval,
+ &loopOpClauseList, iv,
+ /*outer=*/false, &dsp);
+}
+
+static void createWsLoop(Fortran::lower::AbstractConverter &converter,
+ Fortran::lower::pft::Evaluation &eval,
+ llvm::omp::Directive ompDirective,
+ const Fortran::parser::OmpClauseList &beginClauseList,
+ const Fortran::parser::OmpClauseList *endClauseList,
+ mlir::Location loc) {
+ fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+ DataSharingProcessor dsp(converter, beginClauseList, eval);
+ dsp.processStep1();
+
+ Fortran::lower::StatementContext stmtCtx;
+ mlir::Value scheduleChunkClauseOperand;
+ llvm::SmallVector<mlir::Value> lowerBound, upperBound, step, reductionVars,
+ linearVars, linearStepVars;
+ llvm::SmallVector<const Fortran::semantics::Symbol *> iv;
+ llvm::SmallVector<mlir::Attribute> reductionDeclSymbols;
mlir::omp::ClauseOrderKindAttr orderClauseOperand;
mlir::omp::ClauseScheduleKindAttr scheduleValClauseOperand;
- mlir::omp::ScheduleModifierAttr scheduleModClauseOperand;
mlir::UnitAttr nowaitClauseOperand, scheduleSimdClauseOperand;
- llvm::SmallVector<mlir::Attribute> reductionDeclSymbols;
- Fortran::lower::StatementContext stmtCtx;
+ mlir::IntegerAttr orderedClauseOperand;
+ mlir::omp::ScheduleModifierAttr scheduleModClauseOperand;
std::size_t loopVarTypeSize;
- llvm::SmallVector<const Fortran::semantics::Symbol *> iv;
+ ClauseProcessor cp(converter, beginClauseList);
+ cp.processCollapse(loc, eval, lowerBound, upperBound, step, iv,
+ loopVarTypeSize);
+ cp.processScheduleChunk(stmtCtx, scheduleChunkClauseOperand);
+ cp.processReduction(loc, reductionVars, reductionDeclSymbols);
+ cp.processTODO<Fortran::parser::OmpClause::Linear,
+ Fortran::parser::OmpClause::Order>(loc, ompDirective);
+
+ // The types of lower bound, upper bound, and step are converted into the
+ // type of the loop variable if necessary.
+ mlir::Type loopVarType = getLoopVarType(converter, loopVarTypeSize);
+ for (unsigned it = 0; it < (unsigned)lowerBound.size(); it++) {
+ lowerBound[it] =
+ firOpBuilder.createConvert(loc, loopVarType, lowerBound[it]);
+ upperBound[it] =
+ firOpBuilder.createConvert(loc, loopVarType, upperBound[it]);
+ step[it] = firOpBuilder.createConvert(loc, loopVarType, step[it]);
+ }
+
+ auto wsLoopOp = firOpBuilder.create<mlir::omp::WsLoopOp>(
+ loc, lowerBound, upperBound, step, linearVars, linearStepVars,
+ reductionVars,
+ reductionDeclSymbols.empty()
+ ? nullptr
+ : mlir::ArrayAttr::get(firOpBuilder.getContext(),
+ reductionDeclSymbols),
+ scheduleValClauseOperand, scheduleChunkClauseOperand,
+ /*schedule_modifiers=*/nullptr,
+ /*simd_modifier=*/nullptr, nowaitClauseOperand, orderedClauseOperand,
+ orderClauseOperand,
+ /*inclusive=*/firOpBuilder.getUnitAttr());
+
+ // Handle attribute based clauses.
+ if (cp.processOrdered(orderedClauseOperand))
+ wsLoopOp.setOrderedValAttr(orderedClauseOperand);
+
+ if (cp.processSchedule(scheduleValClauseOperand, scheduleModClauseOperand,
+ scheduleSimdClauseOperand)) {
+ wsLoopOp.setScheduleValAttr(scheduleValClauseOperand);
+ wsLoopOp.setScheduleModifierAttr(scheduleModClauseOperand);
+ wsLoopOp.setSimdModifierAttr(scheduleSimdClauseOperand);
+ }
+ // In FORTRAN `nowait` clause occur at the end of `omp do` directive.
+ // i.e
+ // !$omp do
+ // <...>
+ // !$omp end do nowait
+ if (endClauseList) {
+ if (ClauseProcessor(converter, *endClauseList)
+ .processNowait(nowaitClauseOperand))
+ wsLoopOp.setNowaitAttr(nowaitClauseOperand);
+ }
+
+ createBodyOfOp<mlir::omp::WsLoopOp>(wsLoopOp, converter, loc, eval,
+ &beginClauseList, iv,
+ /*outer=*/false, &dsp);
+}
+
+static void genOMP(Fortran::lower::AbstractConverter &converter,
+ Fortran::lower::pft::Evaluation &eval,
+ Fortran::semantics::SemanticsContext &semanticsContext,
+ const Fortran::parser::OpenMPLoopConstruct &loopConstruct) {
const auto &beginLoopDirective =
std::get<Fortran::parser::OmpBeginLoopDirective>(loopConstruct.t);
const auto &loopOpClauseList =
@@ -2995,6 +3121,17 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
const auto ompDirective =
std::get<Fortran::parser::OmpLoopDirective>(beginLoopDirective.t).v;
+ const auto *endClauseList = [&]() {
+ using RetTy = const Fortran::parser::OmpClauseList *;
+ if (auto &endLoopDirective =
+ std::get<std::optional<Fortran::parser::OmpEndLoopDirective>>(
+ loopConstruct.t)) {
+ return RetTy(
+ &std::get<Fortran::parser::OmpClauseList>((*endLoopDirective).t));
+ }
+ return RetTy();
+ }();
+
bool validDirective = false;
if (llvm::omp::topTaskloopSet.test(ompDirective)) {
validDirective = true;
@@ -3033,97 +3170,14 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
")");
}
- DataSharingProcessor dsp(converter, loopOpClauseList, eval);
- dsp.processStep1();
-
- ClauseProcessor cp(converter, loopOpClauseList);
- cp.processCollapse(currentLocation, eval, lowerBound, upperBound, step, iv,
- loopVarTypeSize);
- cp.processScheduleChunk(stmtCtx, scheduleChunkClauseOperand);
- cp.processReduction(currentLocation, reductionVars, reductionDeclSymbols);
- cp.processTODO<Fortran::parser::OmpClause::Linear,
- Fortran::parser::OmpClause::Order>(currentLocation,
- ompDirective);
-
- // The types of lower bound, upper bound, and step are converted into the
- // type of the loop variable if necessary.
- mlir::Type loopVarType = getLoopVarType(converter, loopVarTypeSize);
- for (unsigned it = 0; it < (unsigned)lowerBound.size(); it++) {
- lowerBound[it] = firOpBuilder.createConvert(currentLocation, loopVarType,
- lowerBound[it]);
- upperBound[it] = firOpBuilder.createConvert(currentLocation, loopVarType,
- upperBound[it]);
- step[it] =
- firOpBuilder.createConvert(currentLocation, loopVarType, step[it]);
- }
-
// 2.9.3.1 SIMD construct
if (llvm::omp::allSimdSet.test(ompDirective)) {
- llvm::SmallVector<mlir::Value> alignedVars, nontemporalVars;
- mlir::Value ifClauseOperand;
- mlir::IntegerAttr simdlenClauseOperand, safelenClauseOperand;
- cp.processIf(Fortran::parser::OmpIfClause::DirectiveNameModifier::Simd,
- ifClauseOperand);
- cp.processSimdlen(simdlenClauseOperand);
- cp.processSafelen(safelenClauseOperand);
- cp.processTODO<Fortran::parser::OmpClause::Aligned,
- Fortran::parser::OmpClause::Allocate,
- Fortran::parser::OmpClause::Nontemporal>(currentLocation,
- ompDirective);
-
- mlir::TypeRange resultType;
- auto simdLoopOp = firOpBuilder.create<mlir::omp::SimdLoopOp>(
- currentLocation, resultType, lowerBound, upperBound, step, alignedVars,
- /*alignment_values=*/nullptr, ifClauseOperand, nontemporalVars,
- orderClauseOperand, simdlenClauseOperand, safelenClauseOperand,
- /*inclusive=*/firOpBuilder.getUnitAttr());
- createBodyOfOp<mlir::omp::SimdLoopOp>(
- simdLoopOp, converter, currentLocation, eval, &loopOpClauseList, iv,
- /*outer=*/false, &dsp);
- return;
- }
-
- auto wsLoopOp = firOpBuilder.create<mlir::omp::WsLoopOp>(
- currentLocation, lowerBound, upperBound, step, linearVars, linearStepVars,
- reductionVars,
- reductionDeclSymbols.empty()
- ? nullptr
- : mlir::ArrayAttr::get(firOpBuilder.getContext(),
- reductionDeclSymbols),
- scheduleValClauseOperand, scheduleChunkClauseOperand,
- /*schedule_modifiers=*/nullptr,
- /*simd_modifier=*/nullptr, nowaitClauseOperand, orderedClauseOperand,
- orderClauseOperand,
- /*inclusive=*/firOpBuilder.getUnitAttr());
-
- // Handle attribute based clauses.
- if (cp.processOrdered(orderedClauseOperand))
- wsLoopOp.setOrderedValAttr(orderedClauseOperand);
-
- if (cp.processSchedule(scheduleValClauseOperand, scheduleModClauseOperand,
- scheduleSimdClauseOperand)) {
- wsLoopOp.setScheduleValAttr(scheduleValClauseOperand);
- wsLoopOp.setScheduleModifierAttr(scheduleModClauseOperand);
- wsLoopOp.setSimdModifierAttr(scheduleSimdClauseOperand);
- }
- // In FORTRAN `nowait` clause occur at the end of `omp do` directive.
- // i.e
- // !$omp do
- // <...>
- // !$omp end do nowait
- if (const auto &endClauseList =
- std::get<std::optional<Fortran::parser::OmpEndLoopDirective>>(
- loopConstruct.t)) {
- const auto &clauseList =
- std::get<Fortran::parser::OmpClauseList>((*endClauseList).t);
- if (ClauseProcessor(converter, clauseList)
- .processNowait(nowaitClauseOperand))
- wsLoopOp.setNowaitAttr(nowaitClauseOperand);
+ createSimdLoop(converter, eval, ompDirective, loopOpClauseList,
+ currentLocation);
+ } else {
+ createWsLoop(converter, eval, ompDirective, loopOpClauseList, endClauseList,
+ currentLocation);
}
-
- createBodyOfOp<mlir::omp::WsLoopOp>(wsLoopOp, converter, currentLocation,
- eval, &loopOpClauseList, iv,
- /*outer=*/false, &dsp);
}
static void
>From 841ae5f68c36628bf209b220595fcb6e81bad18d Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 9 Jan 2024 12:22:06 -0600
Subject: [PATCH 02/11] [Flang][OpenMP] Push genEval calls to individual
operations, NFC
Introduce `genNestedEvaluations` that will lower all evaluations
nested in the given, accouting for a potential COLLAPSE directive.
Recursive lowering [2/5]
---
flang/lib/Lower/OpenMP.cpp | 115 +++++++++++++++++++------------------
1 file changed, 60 insertions(+), 55 deletions(-)
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 350cb29121da93..496b4ba27a0533 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -110,6 +110,32 @@ static void gatherFuncAndVarSyms(
}
}
+static Fortran::lower::pft::Evaluation *
+getEvalPastCollapse(Fortran::lower::pft::Evaluation &eval, int collapseValue) {
+ if (collapseValue == 0)
+ return &eval;
+
+ Fortran::lower::pft::Evaluation *curEval = &eval.getFirstNestedEvaluation();
+ for (int i = 1; i < collapseValue; i++) {
+ // The nested evaluations should be DoConstructs (i.e. they should form
+ // a loop nest). Each DoConstruct is a tuple <NonLabelDoStmt, Block,
+ // EndDoStmt>.
+ assert(curEval->isA<Fortran::parser::DoConstruct>());
+ curEval = &*std::next(curEval->getNestedEvaluations().begin());
+ }
+ return curEval;
+}
+
+static void genNestedEvaluations(Fortran::lower::AbstractConverter &converter,
+ Fortran::lower::pft::Evaluation &eval,
+ int collapseValue = 0) {
+ Fortran::lower::pft::Evaluation *curEval =
+ getEvalPastCollapse(eval, collapseValue);
+
+ for (Fortran::lower::pft::Evaluation &e : curEval->getNestedEvaluations())
+ converter.genEval(e);
+}
+
//===----------------------------------------------------------------------===//
// DataSharingProcessor
//===----------------------------------------------------------------------===//
@@ -2944,7 +2970,7 @@ genOmpFlush(Fortran::lower::AbstractConverter &converter,
static void
genOMP(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::SymMap &symTable, Fortran::lower::pft::Evaluation &eval,
Fortran::semantics::SemanticsContext &semanticsContext,
const Fortran::parser::OpenMPStandaloneConstruct &standaloneConstruct) {
std::visit(
@@ -3025,6 +3051,9 @@ createSimdLoop(Fortran::lower::AbstractConverter &converter,
createBodyOfOp<mlir::omp::SimdLoopOp>(simdLoopOp, converter, loc, eval,
&loopOpClauseList, iv,
/*outer=*/false, &dsp);
+
+ genNestedEvaluations(converter, eval,
+ Fortran::lower::getCollapseValue(loopOpClauseList));
}
static void createWsLoop(Fortran::lower::AbstractConverter &converter,
@@ -3106,9 +3135,13 @@ static void createWsLoop(Fortran::lower::AbstractConverter &converter,
createBodyOfOp<mlir::omp::WsLoopOp>(wsLoopOp, converter, loc, eval,
&beginClauseList, iv,
/*outer=*/false, &dsp);
+
+ genNestedEvaluations(converter, eval,
+ Fortran::lower::getCollapseValue(beginClauseList));
}
static void genOMP(Fortran::lower::AbstractConverter &converter,
+ Fortran::lower::SymMap &symTable,
Fortran::lower::pft::Evaluation &eval,
Fortran::semantics::SemanticsContext &semanticsContext,
const Fortran::parser::OpenMPLoopConstruct &loopConstruct) {
@@ -3178,11 +3211,12 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
createWsLoop(converter, eval, ompDirective, loopOpClauseList, endClauseList,
currentLocation);
}
+ genOpenMPReduction(converter, loopOpClauseList);
}
static void
genOMP(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::SymMap &symTable, Fortran::lower::pft::Evaluation &eval,
Fortran::semantics::SemanticsContext &semanticsContext,
const Fortran::parser::OpenMPBlockConstruct &blockConstruct) {
const auto &beginBlockDirective =
@@ -3297,11 +3331,14 @@ genOMP(Fortran::lower::AbstractConverter &converter,
break;
}
}
+
+ genNestedEvaluations(converter, eval);
+ genOpenMPReduction(converter, beginClauseList);
}
static void
genOMP(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::SymMap &symTable, Fortran::lower::pft::Evaluation &eval,
const Fortran::parser::OpenMPCriticalConstruct &criticalConstruct) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
mlir::Location currentLocation = converter.getCurrentLocation();
@@ -3335,11 +3372,12 @@ genOMP(Fortran::lower::AbstractConverter &converter,
}();
createBodyOfOp<mlir::omp::CriticalOp>(criticalOp, converter, currentLocation,
eval);
+ genNestedEvaluations(converter, eval);
}
static void
genOMP(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::SymMap &symTable, Fortran::lower::pft::Evaluation &eval,
const Fortran::parser::OpenMPSectionConstruct §ionConstruct) {
mlir::Location currentLocation = converter.getCurrentLocation();
const Fortran::parser::OpenMPConstruct *parentOmpConstruct =
@@ -3358,14 +3396,17 @@ genOMP(Fortran::lower::AbstractConverter &converter,
.t);
// Currently only private/firstprivate clause is handled, and
// all privatization is done within `omp.section` operations.
+ symTable.pushScope();
genOpWithBody<mlir::omp::SectionOp>(converter, eval, currentLocation,
/*outerCombined=*/false,
§ionsClauseList);
+ genNestedEvaluations(converter, eval);
+ symTable.popScope();
}
static void
genOMP(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::SymMap &symTable, Fortran::lower::pft::Evaluation &eval,
const Fortran::parser::OpenMPSectionsConstruct §ionsConstruct) {
mlir::Location currentLocation = converter.getCurrentLocation();
llvm::SmallVector<mlir::Value> allocateOperands, allocatorOperands;
@@ -3405,11 +3446,13 @@ genOMP(Fortran::lower::AbstractConverter &converter,
/*reduction_vars=*/mlir::ValueRange(),
/*reductions=*/nullptr, allocateOperands,
allocatorOperands, nowaitClauseOperand);
+
+ genNestedEvaluations(converter, eval);
}
static void
genOMP(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::SymMap &symTable, Fortran::lower::pft::Evaluation &eval,
const Fortran::parser::OpenMPAtomicConstruct &atomicConstruct) {
std::visit(
Fortran::common::visitors{
@@ -3503,6 +3546,7 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
}
static void genOMP(Fortran::lower::AbstractConverter &converter,
+ Fortran::lower::SymMap &symTable,
Fortran::semantics::SemanticsContext &semanticsContext,
Fortran::lower::pft::Evaluation &eval,
const Fortran::parser::OpenMPConstruct &ompConstruct) {
@@ -3510,17 +3554,18 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
Fortran::common::visitors{
[&](const Fortran::parser::OpenMPStandaloneConstruct
&standaloneConstruct) {
- genOMP(converter, eval, semanticsContext, standaloneConstruct);
+ genOMP(converter, symTable, eval, semanticsContext,
+ standaloneConstruct);
},
[&](const Fortran::parser::OpenMPSectionsConstruct
§ionsConstruct) {
- genOMP(converter, eval, sectionsConstruct);
+ genOMP(converter, symTable, eval, sectionsConstruct);
},
[&](const Fortran::parser::OpenMPSectionConstruct §ionConstruct) {
- genOMP(converter, eval, sectionConstruct);
+ genOMP(converter, symTable, eval, sectionConstruct);
},
[&](const Fortran::parser::OpenMPLoopConstruct &loopConstruct) {
- genOMP(converter, eval, semanticsContext, loopConstruct);
+ genOMP(converter, symTable, eval, semanticsContext, loopConstruct);
},
[&](const Fortran::parser::OpenMPDeclarativeAllocate
&execAllocConstruct) {
@@ -3535,14 +3580,14 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
TODO(converter.getCurrentLocation(), "OpenMPAllocatorsConstruct");
},
[&](const Fortran::parser::OpenMPBlockConstruct &blockConstruct) {
- genOMP(converter, eval, semanticsContext, blockConstruct);
+ genOMP(converter, symTable, eval, semanticsContext, blockConstruct);
},
[&](const Fortran::parser::OpenMPAtomicConstruct &atomicConstruct) {
- genOMP(converter, eval, atomicConstruct);
+ genOMP(converter, symTable, eval, atomicConstruct);
},
[&](const Fortran::parser::OpenMPCriticalConstruct
&criticalConstruct) {
- genOMP(converter, eval, criticalConstruct);
+ genOMP(converter, symTable, eval, criticalConstruct);
},
},
ompConstruct.u);
@@ -3606,47 +3651,8 @@ void Fortran::lower::genOpenMPConstruct(
Fortran::semantics::SemanticsContext &semanticsContext,
Fortran::lower::pft::Evaluation &eval,
const Fortran::parser::OpenMPConstruct &omp) {
-
symTable.pushScope();
- genOMP(converter, semanticsContext, eval, omp);
-
- const Fortran::parser::OpenMPLoopConstruct *ompLoop =
- std::get_if<Fortran::parser::OpenMPLoopConstruct>(&omp.u);
- const Fortran::parser::OpenMPBlockConstruct *ompBlock =
- std::get_if<Fortran::parser::OpenMPBlockConstruct>(&omp.u);
-
- // If loop is part of an OpenMP Construct then the OpenMP dialect
- // workshare loop operation has already been created. Only the
- // body needs to be created here and the do_loop can be skipped.
- // Skip the number of collapsed loops, which is 1 when there is a
- // no collapse requested.
-
- Fortran::lower::pft::Evaluation *curEval = &eval;
- const Fortran::parser::OmpClauseList *loopOpClauseList = nullptr;
- if (ompLoop) {
- loopOpClauseList = &std::get<Fortran::parser::OmpClauseList>(
- std::get<Fortran::parser::OmpBeginLoopDirective>(ompLoop->t).t);
- int64_t collapseValue = Fortran::lower::getCollapseValue(*loopOpClauseList);
-
- curEval = &curEval->getFirstNestedEvaluation();
- for (int64_t i = 1; i < collapseValue; i++) {
- curEval = &*std::next(curEval->getNestedEvaluations().begin());
- }
- }
-
- for (Fortran::lower::pft::Evaluation &e : curEval->getNestedEvaluations())
- converter.genEval(e);
-
- if (ompLoop) {
- genOpenMPReduction(converter, *loopOpClauseList);
- } else if (ompBlock) {
- const auto &blockStart =
- std::get<Fortran::parser::OmpBeginBlockDirective>(ompBlock->t);
- const auto &blockClauses =
- std::get<Fortran::parser::OmpClauseList>(blockStart.t);
- genOpenMPReduction(converter, blockClauses);
- }
-
+ genOMP(converter, symTable, semanticsContext, eval, omp);
symTable.popScope();
}
@@ -3655,8 +3661,7 @@ void Fortran::lower::genOpenMPDeclarativeConstruct(
Fortran::lower::pft::Evaluation &eval,
const Fortran::parser::OpenMPDeclarativeConstruct &omp) {
genOMP(converter, eval, omp);
- for (Fortran::lower::pft::Evaluation &e : eval.getNestedEvaluations())
- converter.genEval(e);
+ genNestedEvaluations(converter, eval);
}
void Fortran::lower::genOpenMPSymbolProperties(
>From 67ea0e5df3c4919030c963c969d8f9cda0b5996b Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 9 Jan 2024 12:25:10 -0600
Subject: [PATCH 03/11] [Flang][OpenMP] Handle SECTION construct from within
SECTIONS
Introduce `createSectionOp`, invoke it from the SECTIONS construct for
each nested SECTION construct. This makes it unnecessary to embed
OpenMPSectionConstruct inside of OpenMPSectionConstruct anymore.
Recursive lowering [3/5]
---
flang/lib/Lower/OpenMP.cpp | 57 +++++++++++++++++---------------------
1 file changed, 26 insertions(+), 31 deletions(-)
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 496b4ba27a0533..aa315d7ab280e2 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -2379,6 +2379,18 @@ genParallelOp(Fortran::lower::AbstractConverter &converter,
procBindKindAttr);
}
+static mlir::omp::SectionOp
+genSectionOp(Fortran::lower::AbstractConverter &converter,
+ Fortran::lower::pft::Evaluation &eval,
+ mlir::Location currentLocation,
+ const Fortran::parser::OmpClauseList §ionsClauseList) {
+ // Currently only private/firstprivate clause is handled, and
+ // all privatization is done within `omp.section` operations.
+ return genOpWithBody<mlir::omp::SectionOp>(converter, eval, currentLocation,
+ /*outerCombined=*/false,
+ §ionsClauseList);
+}
+
static mlir::omp::SingleOp
genSingleOp(Fortran::lower::AbstractConverter &converter,
Fortran::lower::pft::Evaluation &eval,
@@ -3375,35 +3387,6 @@ genOMP(Fortran::lower::AbstractConverter &converter,
genNestedEvaluations(converter, eval);
}
-static void
-genOMP(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::SymMap &symTable, Fortran::lower::pft::Evaluation &eval,
- const Fortran::parser::OpenMPSectionConstruct §ionConstruct) {
- mlir::Location currentLocation = converter.getCurrentLocation();
- const Fortran::parser::OpenMPConstruct *parentOmpConstruct =
- eval.parentConstruct->getIf<Fortran::parser::OpenMPConstruct>();
- assert(parentOmpConstruct &&
- "No enclosing parent OpenMPConstruct on SECTION construct");
- const Fortran::parser::OpenMPSectionsConstruct *sectionsConstruct =
- std::get_if<Fortran::parser::OpenMPSectionsConstruct>(
- &parentOmpConstruct->u);
- assert(sectionsConstruct && "SECTION construct must have parent"
- "SECTIONS construct");
- const Fortran::parser::OmpClauseList §ionsClauseList =
- std::get<Fortran::parser::OmpClauseList>(
- std::get<Fortran::parser::OmpBeginSectionsDirective>(
- sectionsConstruct->t)
- .t);
- // Currently only private/firstprivate clause is handled, and
- // all privatization is done within `omp.section` operations.
- symTable.pushScope();
- genOpWithBody<mlir::omp::SectionOp>(converter, eval, currentLocation,
- /*outerCombined=*/false,
- §ionsClauseList);
- genNestedEvaluations(converter, eval);
- symTable.popScope();
-}
-
static void
genOMP(Fortran::lower::AbstractConverter &converter,
Fortran::lower::SymMap &symTable, Fortran::lower::pft::Evaluation &eval,
@@ -3447,7 +3430,18 @@ genOMP(Fortran::lower::AbstractConverter &converter,
/*reductions=*/nullptr, allocateOperands,
allocatorOperands, nowaitClauseOperand);
- genNestedEvaluations(converter, eval);
+ const auto §ionBlocks =
+ std::get<Fortran::parser::OmpSectionBlocks>(sectionsConstruct.t);
+ auto &firOpBuilder = converter.getFirOpBuilder();
+ auto ip = firOpBuilder.saveInsertionPoint();
+ for (const auto &[nblock, neval] :
+ llvm::zip(sectionBlocks.v, eval.getNestedEvaluations())) {
+ symTable.pushScope();
+ genSectionOp(converter, neval, currentLocation, sectionsClauseList);
+ genNestedEvaluations(converter, neval);
+ symTable.popScope();
+ firOpBuilder.restoreInsertionPoint(ip);
+ }
}
static void
@@ -3562,7 +3556,8 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
genOMP(converter, symTable, eval, sectionsConstruct);
},
[&](const Fortran::parser::OpenMPSectionConstruct §ionConstruct) {
- genOMP(converter, symTable, eval, sectionConstruct);
+ // SECTION constructs are handled as a part of SECTIONS.
+ llvm_unreachable("Unexpected standalone OMP SECTION");
},
[&](const Fortran::parser::OpenMPLoopConstruct &loopConstruct) {
genOMP(converter, symTable, eval, semanticsContext, loopConstruct);
>From 883afd9e93c34ffb05fb54cab6cf7ca0cb3379b6 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 8 Jan 2024 15:53:07 -0600
Subject: [PATCH 04/11] [Flang][OpenMP] Push genEval closer to leaf lowering
functions
Recursive lowering [4/5]
---
flang/lib/Lower/OpenMP.cpp | 226 +++++++++++++++++++++----------------
1 file changed, 130 insertions(+), 96 deletions(-)
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index aa315d7ab280e2..6e93387fbff7cc 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -2178,7 +2178,7 @@ createAndSetPrivatizedLoopVar(Fortran::lower::AbstractConverter &converter,
template <typename Op>
static void createBodyOfOp(
Op &op, Fortran::lower::AbstractConverter &converter, mlir::Location &loc,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
const Fortran::parser::OmpClauseList *clauses = nullptr,
const llvm::SmallVector<const Fortran::semantics::Symbol *> &args = {},
bool outerCombined = false, DataSharingProcessor *dsp = nullptr) {
@@ -2248,11 +2248,15 @@ static void createBodyOfOp(
if (clauses)
ClauseProcessor(converter, *clauses).processCopyin();
}
+
+ if (genNested)
+ genNestedEvaluations(converter, eval);
}
static void genBodyOfTargetDataOp(
Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval, mlir::omp::DataOp &dataOp,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
+ mlir::omp::DataOp &dataOp,
const llvm::SmallVector<mlir::Type> &useDeviceTypes,
const llvm::SmallVector<mlir::Location> &useDeviceLocs,
const llvm::SmallVector<const Fortran::semantics::Symbol *>
@@ -2310,26 +2314,30 @@ static void genBodyOfTargetDataOp(
// Set the insertion point after the marker.
firOpBuilder.setInsertionPointAfter(undefMarker.getDefiningOp());
+ if (genNested)
+ genNestedEvaluations(converter, eval);
}
template <typename OpTy, typename... Args>
static OpTy genOpWithBody(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
mlir::Location currentLocation, bool outerCombined,
const Fortran::parser::OmpClauseList *clauseList,
Args &&...args) {
auto op = converter.getFirOpBuilder().create<OpTy>(
currentLocation, std::forward<Args>(args)...);
- createBodyOfOp<OpTy>(op, converter, currentLocation, eval, clauseList,
+ createBodyOfOp<OpTy>(op, converter, currentLocation, eval, genNested,
+ clauseList,
/*args=*/{}, outerCombined);
return op;
}
static mlir::omp::MasterOp
genMasterOp(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
mlir::Location currentLocation) {
- return genOpWithBody<mlir::omp::MasterOp>(converter, eval, currentLocation,
+ return genOpWithBody<mlir::omp::MasterOp>(converter, eval, genNested,
+ currentLocation,
/*outerCombined=*/false,
/*clauseList=*/nullptr,
/*resultTypes=*/mlir::TypeRange());
@@ -2337,16 +2345,17 @@ genMasterOp(Fortran::lower::AbstractConverter &converter,
static mlir::omp::OrderedRegionOp
genOrderedRegionOp(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
mlir::Location currentLocation) {
return genOpWithBody<mlir::omp::OrderedRegionOp>(
- converter, eval, currentLocation, /*outerCombined=*/false,
+ converter, eval, genNested, currentLocation,
+ /*outerCombined=*/false,
/*clauseList=*/nullptr, /*simd=*/false);
}
static mlir::omp::ParallelOp
genParallelOp(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
mlir::Location currentLocation,
const Fortran::parser::OmpClauseList &clauseList,
bool outerCombined = false) {
@@ -2368,7 +2377,7 @@ genParallelOp(Fortran::lower::AbstractConverter &converter,
cp.processReduction(currentLocation, reductionVars, reductionDeclSymbols);
return genOpWithBody<mlir::omp::ParallelOp>(
- converter, eval, currentLocation, outerCombined, &clauseList,
+ converter, eval, genNested, currentLocation, outerCombined, &clauseList,
/*resultTypes=*/mlir::TypeRange(), ifClauseOperand,
numThreadsClauseOperand, allocateOperands, allocatorOperands,
reductionVars,
@@ -2381,19 +2390,19 @@ genParallelOp(Fortran::lower::AbstractConverter &converter,
static mlir::omp::SectionOp
genSectionOp(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
mlir::Location currentLocation,
const Fortran::parser::OmpClauseList §ionsClauseList) {
// Currently only private/firstprivate clause is handled, and
// all privatization is done within `omp.section` operations.
- return genOpWithBody<mlir::omp::SectionOp>(converter, eval, currentLocation,
- /*outerCombined=*/false,
- §ionsClauseList);
+ return genOpWithBody<mlir::omp::SectionOp>(
+ converter, eval, genNested, currentLocation,
+ /*outerCombined=*/false, §ionsClauseList);
}
static mlir::omp::SingleOp
genSingleOp(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
mlir::Location currentLocation,
const Fortran::parser::OmpClauseList &beginClauseList,
const Fortran::parser::OmpClauseList &endClauseList) {
@@ -2408,13 +2417,15 @@ genSingleOp(Fortran::lower::AbstractConverter &converter,
ClauseProcessor(converter, endClauseList).processNowait(nowaitAttr);
return genOpWithBody<mlir::omp::SingleOp>(
- converter, eval, currentLocation, /*outerCombined=*/false,
- &beginClauseList, allocateOperands, allocatorOperands, nowaitAttr);
+ converter, eval, genNested, currentLocation,
+ /*outerCombined=*/false, &beginClauseList, allocateOperands,
+ allocatorOperands, nowaitAttr);
}
static mlir::omp::TaskOp
genTaskOp(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval, mlir::Location currentLocation,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
+ mlir::Location currentLocation,
const Fortran::parser::OmpClauseList &clauseList) {
Fortran::lower::StatementContext stmtCtx;
mlir::Value ifClauseOperand, finalClauseOperand, priorityClauseOperand;
@@ -2439,8 +2450,9 @@ genTaskOp(Fortran::lower::AbstractConverter &converter,
currentLocation, llvm::omp::Directive::OMPD_task);
return genOpWithBody<mlir::omp::TaskOp>(
- converter, eval, currentLocation, /*outerCombined=*/false, &clauseList,
- ifClauseOperand, finalClauseOperand, untiedAttr, mergeableAttr,
+ converter, eval, genNested, currentLocation,
+ /*outerCombined=*/false, &clauseList, ifClauseOperand, finalClauseOperand,
+ untiedAttr, mergeableAttr,
/*in_reduction_vars=*/mlir::ValueRange(),
/*in_reductions=*/nullptr, priorityClauseOperand,
dependTypeOperands.empty()
@@ -2452,7 +2464,7 @@ genTaskOp(Fortran::lower::AbstractConverter &converter,
static mlir::omp::TaskGroupOp
genTaskGroupOp(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
mlir::Location currentLocation,
const Fortran::parser::OmpClauseList &clauseList) {
llvm::SmallVector<mlir::Value> allocateOperands, allocatorOperands;
@@ -2461,7 +2473,8 @@ genTaskGroupOp(Fortran::lower::AbstractConverter &converter,
cp.processTODO<Fortran::parser::OmpClause::TaskReduction>(
currentLocation, llvm::omp::Directive::OMPD_taskgroup);
return genOpWithBody<mlir::omp::TaskGroupOp>(
- converter, eval, currentLocation, /*outerCombined=*/false, &clauseList,
+ converter, eval, genNested, currentLocation,
+ /*outerCombined=*/false, &clauseList,
/*task_reduction_vars=*/mlir::ValueRange(),
/*task_reductions=*/nullptr, allocateOperands, allocatorOperands);
}
@@ -2470,7 +2483,7 @@ static mlir::omp::DataOp
genDataOp(Fortran::lower::AbstractConverter &converter,
Fortran::lower::pft::Evaluation &eval,
Fortran::semantics::SemanticsContext &semanticsContext,
- mlir::Location currentLocation,
+ bool genNested, mlir::Location currentLocation,
const Fortran::parser::OmpClauseList &clauseList) {
Fortran::lower::StatementContext stmtCtx;
mlir::Value ifClauseOperand, deviceOperand;
@@ -2494,8 +2507,8 @@ genDataOp(Fortran::lower::AbstractConverter &converter,
auto dataOp = converter.getFirOpBuilder().create<mlir::omp::DataOp>(
currentLocation, ifClauseOperand, deviceOperand, devicePtrOperands,
deviceAddrOperands, mapOperands);
- genBodyOfTargetDataOp(converter, eval, dataOp, useDeviceTypes, useDeviceLocs,
- useDeviceSymbols, currentLocation);
+ genBodyOfTargetDataOp(converter, eval, genNested, dataOp, useDeviceTypes,
+ useDeviceLocs, useDeviceSymbols, currentLocation);
return dataOp;
}
@@ -2556,7 +2569,8 @@ genEnterExitUpdateDataOp(Fortran::lower::AbstractConverter &converter,
// all the symbols present in mapSymbols as block arguments to this block.
static void genBodyOfTargetOp(
Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval, mlir::omp::TargetOp &targetOp,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
+ mlir::omp::TargetOp &targetOp,
const llvm::SmallVector<mlir::Type> &mapSymTypes,
const llvm::SmallVector<mlir::Location> &mapSymLocs,
const llvm::SmallVector<const Fortran::semantics::Symbol *> &mapSymbols,
@@ -2686,7 +2700,6 @@ static void genBodyOfTargetOp(
// Create blocks for unstructured regions. This has to be done since
// blocks are initially allocated with the function as the parent region.
- // the parent region of blocks.
if (eval.lowerAsUnstructured()) {
Fortran::lower::createEmptyRegionBlocks<mlir::omp::TerminatorOp,
mlir::omp::YieldOp>(
@@ -2697,13 +2710,15 @@ static void genBodyOfTargetOp(
// Create the insertion point after the marker.
firOpBuilder.setInsertionPointAfter(undefMarker.getDefiningOp());
+ if (genNested)
+ genNestedEvaluations(converter, eval);
}
static mlir::omp::TargetOp
genTargetOp(Fortran::lower::AbstractConverter &converter,
Fortran::lower::pft::Evaluation &eval,
Fortran::semantics::SemanticsContext &semanticsContext,
- mlir::Location currentLocation,
+ bool genNested, mlir::Location currentLocation,
const Fortran::parser::OmpClauseList &clauseList,
llvm::omp::Directive directive, bool outerCombined = false) {
Fortran::lower::StatementContext stmtCtx;
@@ -2803,15 +2818,15 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
currentLocation, ifClauseOperand, deviceOperand, threadLimitOperand,
nowaitAttr, mapOperands);
- genBodyOfTargetOp(converter, eval, targetOp, mapSymTypes, mapSymLocs,
- mapSymbols, currentLocation);
+ genBodyOfTargetOp(converter, eval, genNested, targetOp, mapSymTypes,
+ mapSymLocs, mapSymbols, currentLocation);
return targetOp;
}
static mlir::omp::TeamsOp
genTeamsOp(Fortran::lower::AbstractConverter &converter,
- Fortran::lower::pft::Evaluation &eval,
+ Fortran::lower::pft::Evaluation &eval, bool genNested,
mlir::Location currentLocation,
const Fortran::parser::OmpClauseList &clauseList,
bool outerCombined = false) {
@@ -2832,7 +2847,7 @@ genTeamsOp(Fortran::lower::AbstractConverter &converter,
currentLocation, llvm::omp::Directive::OMPD_teams);
return genOpWithBody<mlir::omp::TeamsOp>(
- converter, eval, currentLocation, outerCombined, &clauseList,
+ converter, eval, genNested, currentLocation, outerCombined, &clauseList,
/*num_teams_lower=*/nullptr, numTeamsClauseOperand, ifClauseOperand,
threadLimitClauseOperand, allocateOperands, allocatorOperands,
reductionVars,
@@ -2916,6 +2931,7 @@ static void
genOmpSimpleStandalone(Fortran::lower::AbstractConverter &converter,
Fortran::lower::pft::Evaluation &eval,
Fortran::semantics::SemanticsContext &semanticsContext,
+ bool genNested,
const Fortran::parser::OpenMPSimpleStandaloneConstruct
&simpleStandaloneConstruct) {
const auto &directive =
@@ -2943,7 +2959,8 @@ genOmpSimpleStandalone(Fortran::lower::AbstractConverter &converter,
firOpBuilder.create<mlir::omp::TaskyieldOp>(currentLocation);
break;
case llvm::omp::Directive::OMPD_target_data:
- genDataOp(converter, eval, semanticsContext, currentLocation, opClauseList);
+ genDataOp(converter, eval, semanticsContext, genNested, currentLocation,
+ opClauseList);
break;
case llvm::omp::Directive::OMPD_target_enter_data:
genEnterExitUpdateDataOp<mlir::omp::EnterDataOp>(
@@ -2990,6 +3007,7 @@ genOMP(Fortran::lower::AbstractConverter &converter,
[&](const Fortran::parser::OpenMPSimpleStandaloneConstruct
&simpleStandaloneConstruct) {
genOmpSimpleStandalone(converter, eval, semanticsContext,
+ /*genNested=*/true,
simpleStandaloneConstruct);
},
[&](const Fortran::parser::OpenMPFlushConstruct &flushConstruct) {
@@ -3060,12 +3078,12 @@ createSimdLoop(Fortran::lower::AbstractConverter &converter,
/*alignment_values=*/nullptr, ifClauseOperand, nontemporalVars,
orderClauseOperand, simdlenClauseOperand, safelenClauseOperand,
/*inclusive=*/firOpBuilder.getUnitAttr());
- createBodyOfOp<mlir::omp::SimdLoopOp>(simdLoopOp, converter, loc, eval,
- &loopOpClauseList, iv,
- /*outer=*/false, &dsp);
- genNestedEvaluations(converter, eval,
- Fortran::lower::getCollapseValue(loopOpClauseList));
+ auto *nestedEval = getEvalPastCollapse(
+ eval, Fortran::lower::getCollapseValue(loopOpClauseList));
+ createBodyOfOp<mlir::omp::SimdLoopOp>(simdLoopOp, converter, loc, *nestedEval,
+ /*genNested=*/true, &loopOpClauseList,
+ iv, /*outer=*/false, &dsp);
}
static void createWsLoop(Fortran::lower::AbstractConverter &converter,
@@ -3144,12 +3162,11 @@ static void createWsLoop(Fortran::lower::AbstractConverter &converter,
wsLoopOp.setNowaitAttr(nowaitClauseOperand);
}
- createBodyOfOp<mlir::omp::WsLoopOp>(wsLoopOp, converter, loc, eval,
- &beginClauseList, iv,
+ auto *nestedEval = getEvalPastCollapse(
+ eval, Fortran::lower::getCollapseValue(beginClauseList));
+ createBodyOfOp<mlir::omp::WsLoopOp>(wsLoopOp, converter, loc, *nestedEval,
+ /*genNested=*/true, &beginClauseList, iv,
/*outer=*/false, &dsp);
-
- genNestedEvaluations(converter, eval,
- Fortran::lower::getCollapseValue(beginClauseList));
}
static void genOMP(Fortran::lower::AbstractConverter &converter,
@@ -3186,13 +3203,15 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
if ((llvm::omp::allTargetSet & llvm::omp::loopConstructSet)
.test(ompDirective)) {
validDirective = true;
- genTargetOp(converter, eval, semanticsContext, currentLocation,
- loopOpClauseList, ompDirective, /*outerCombined=*/true);
+ genTargetOp(converter, eval, semanticsContext, /*genNested=*/false,
+ currentLocation, loopOpClauseList, ompDirective,
+ /*outerCombined=*/true);
}
if ((llvm::omp::allTeamsSet & llvm::omp::loopConstructSet)
.test(ompDirective)) {
validDirective = true;
- genTeamsOp(converter, eval, currentLocation, loopOpClauseList,
+ genTeamsOp(converter, eval, /*genNested=*/false, currentLocation,
+ loopOpClauseList,
/*outerCombined=*/true);
}
if (llvm::omp::allDistributeSet.test(ompDirective)) {
@@ -3202,7 +3221,8 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
if ((llvm::omp::allParallelSet & llvm::omp::loopConstructSet)
.test(ompDirective)) {
validDirective = true;
- genParallelOp(converter, eval, currentLocation, loopOpClauseList,
+ genParallelOp(converter, eval, /*genNested=*/false, currentLocation,
+ loopOpClauseList,
/*outerCombined=*/true);
}
}
@@ -3273,76 +3293,89 @@ genOMP(Fortran::lower::AbstractConverter &converter,
TODO(clauseLocation, "OpenMP Block construct clause");
}
+ bool singleDirective = true;
mlir::Location currentLocation = converter.genLocation(directive.source);
switch (directive.v) {
case llvm::omp::Directive::OMPD_master:
- genMasterOp(converter, eval, currentLocation);
+ genMasterOp(converter, eval, /*genNested=*/true, currentLocation);
break;
case llvm::omp::Directive::OMPD_ordered:
- genOrderedRegionOp(converter, eval, currentLocation);
+ genOrderedRegionOp(converter, eval, /*genNested=*/true, currentLocation);
break;
case llvm::omp::Directive::OMPD_parallel:
- genParallelOp(converter, eval, currentLocation, beginClauseList);
+ genParallelOp(converter, eval, /*genNested=*/true, currentLocation,
+ beginClauseList);
break;
case llvm::omp::Directive::OMPD_single:
- genSingleOp(converter, eval, currentLocation, beginClauseList,
- endClauseList);
+ genSingleOp(converter, eval, /*genNested=*/true, currentLocation,
+ beginClauseList, endClauseList);
break;
case llvm::omp::Directive::OMPD_target:
- genTargetOp(converter, eval, semanticsContext, currentLocation,
- beginClauseList, directive.v);
+ genTargetOp(converter, eval, semanticsContext, /*genNested=*/true,
+ currentLocation, beginClauseList, directive.v);
break;
case llvm::omp::Directive::OMPD_target_data:
- genDataOp(converter, eval, semanticsContext, currentLocation,
- beginClauseList);
+ genDataOp(converter, eval, semanticsContext, /*genNested=*/true,
+ currentLocation, beginClauseList);
break;
case llvm::omp::Directive::OMPD_task:
- genTaskOp(converter, eval, currentLocation, beginClauseList);
+ genTaskOp(converter, eval, /*genNested=*/true, currentLocation,
+ beginClauseList);
break;
case llvm::omp::Directive::OMPD_taskgroup:
- genTaskGroupOp(converter, eval, currentLocation, beginClauseList);
+ genTaskGroupOp(converter, eval, /*genNested=*/true, currentLocation,
+ beginClauseList);
break;
case llvm::omp::Directive::OMPD_teams:
- genTeamsOp(converter, eval, currentLocation, beginClauseList,
+ genTeamsOp(converter, eval, /*genNested=*/true, currentLocation,
+ beginClauseList,
/*outerCombined=*/false);
break;
case llvm::omp::Directive::OMPD_workshare:
TODO(currentLocation, "Workshare construct");
break;
- default: {
- // Codegen for combined directives
- bool combinedDirective = false;
- if ((llvm::omp::allTargetSet & llvm::omp::blockConstructSet)
- .test(directive.v)) {
- genTargetOp(converter, eval, semanticsContext, currentLocation,
- beginClauseList, directive.v, /*outerCombined=*/true);
- combinedDirective = true;
- }
- if ((llvm::omp::allTeamsSet & llvm::omp::blockConstructSet)
- .test(directive.v)) {
- genTeamsOp(converter, eval, currentLocation, beginClauseList);
- combinedDirective = true;
- }
- if ((llvm::omp::allParallelSet & llvm::omp::blockConstructSet)
- .test(directive.v)) {
- bool outerCombined =
- directive.v != llvm::omp::Directive::OMPD_target_parallel;
- genParallelOp(converter, eval, currentLocation, beginClauseList,
- outerCombined);
- combinedDirective = true;
- }
- if ((llvm::omp::workShareSet & llvm::omp::blockConstructSet)
- .test(directive.v)) {
- TODO(currentLocation, "Workshare construct");
- combinedDirective = true;
- }
- if (!combinedDirective)
- TODO(currentLocation, "Unhandled block directive (" +
- llvm::omp::getOpenMPDirectiveName(directive.v) +
- ")");
+ default:
+ singleDirective = false;
break;
}
+
+ if (singleDirective) {
+ genOpenMPReduction(converter, beginClauseList);
+ return;
+ }
+
+ // Codegen for combined directives
+ bool combinedDirective = false;
+ if ((llvm::omp::allTargetSet & llvm::omp::blockConstructSet)
+ .test(directive.v)) {
+ genTargetOp(converter, eval, semanticsContext, /*genNested=*/false,
+ currentLocation, beginClauseList, directive.v,
+ /*outerCombined=*/true);
+ combinedDirective = true;
+ }
+ if ((llvm::omp::allTeamsSet & llvm::omp::blockConstructSet)
+ .test(directive.v)) {
+ genTeamsOp(converter, eval, /*genNested=*/false, currentLocation,
+ beginClauseList);
+ combinedDirective = true;
+ }
+ if ((llvm::omp::allParallelSet & llvm::omp::blockConstructSet)
+ .test(directive.v)) {
+ bool outerCombined =
+ directive.v != llvm::omp::Directive::OMPD_target_parallel;
+ genParallelOp(converter, eval, /*genNested=*/false, currentLocation,
+ beginClauseList, outerCombined);
+ combinedDirective = true;
+ }
+ if ((llvm::omp::workShareSet & llvm::omp::blockConstructSet)
+ .test(directive.v)) {
+ TODO(currentLocation, "Workshare construct");
+ combinedDirective = true;
}
+ if (!combinedDirective)
+ TODO(currentLocation, "Unhandled block directive (" +
+ llvm::omp::getOpenMPDirectiveName(directive.v) +
+ ")");
genNestedEvaluations(converter, eval);
genOpenMPReduction(converter, beginClauseList);
@@ -3383,8 +3416,7 @@ genOMP(Fortran::lower::AbstractConverter &converter,
global.getSymName()));
}();
createBodyOfOp<mlir::omp::CriticalOp>(criticalOp, converter, currentLocation,
- eval);
- genNestedEvaluations(converter, eval);
+ eval, /*genNested=*/true);
}
static void
@@ -3411,7 +3443,8 @@ genOMP(Fortran::lower::AbstractConverter &converter,
// Parallel wrapper of PARALLEL SECTIONS construct
if (dir == llvm::omp::Directive::OMPD_parallel_sections) {
- genParallelOp(converter, eval, currentLocation, sectionsClauseList,
+ genParallelOp(converter, eval,
+ /*genNested=*/false, currentLocation, sectionsClauseList,
/*outerCombined=*/true);
} else {
const auto &endSectionsDirective =
@@ -3423,7 +3456,8 @@ genOMP(Fortran::lower::AbstractConverter &converter,
}
// SECTIONS construct
- genOpWithBody<mlir::omp::SectionsOp>(converter, eval, currentLocation,
+ genOpWithBody<mlir::omp::SectionsOp>(converter, eval,
+ /*genNested=*/false, currentLocation,
/*outerCombined=*/false,
/*clauseList=*/nullptr,
/*reduction_vars=*/mlir::ValueRange(),
@@ -3437,8 +3471,8 @@ genOMP(Fortran::lower::AbstractConverter &converter,
for (const auto &[nblock, neval] :
llvm::zip(sectionBlocks.v, eval.getNestedEvaluations())) {
symTable.pushScope();
- genSectionOp(converter, neval, currentLocation, sectionsClauseList);
- genNestedEvaluations(converter, neval);
+ genSectionOp(converter, neval, /*genNested=*/true, currentLocation,
+ sectionsClauseList);
symTable.popScope();
firOpBuilder.restoreInsertionPoint(ip);
}
>From 1b5524ae8874e389d373a55417919afa56beb2b5 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 8 Jan 2024 15:53:07 -0600
Subject: [PATCH 05/11] [Flang][OpenMP] Restructure recursive lowering in
`createBodyOfOp`
This brings `createBodyOfOp` to its final intended form. First, input
privatization is performed, then the recursive lowering takes place,
and finally the output privatization (lastprivate) is done.
This enables fixing a known issue with infinite loops inside of an
OpenMP region, and the fix is included in this patch.
Fixes https://github.com/llvm/llvm-project/issues/74348.
Recursive lowering [5/5]
---
flang/include/flang/Lower/OpenMP.h | 4 +-
flang/lib/Lower/OpenMP.cpp | 133 ++++++++++++------
flang/test/Lower/OpenMP/FIR/sections.f90 | 6 +-
.../OpenMP/infinite-loop-in-construct.f90 | 19 +++
flang/test/Lower/OpenMP/sections.f90 | 6 +-
5 files changed, 119 insertions(+), 49 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/infinite-loop-in-construct.f90
diff --git a/flang/include/flang/Lower/OpenMP.h b/flang/include/flang/Lower/OpenMP.h
index 6e772c43d8c46e..477d3e7d9da3a8 100644
--- a/flang/include/flang/Lower/OpenMP.h
+++ b/flang/include/flang/Lower/OpenMP.h
@@ -50,8 +50,8 @@ struct Variable;
} // namespace pft
// Generate the OpenMP terminator for Operation at Location.
-void genOpenMPTerminator(fir::FirOpBuilder &, mlir::Operation *,
- mlir::Location);
+mlir::Operation *genOpenMPTerminator(fir::FirOpBuilder &, mlir::Operation *,
+ mlir::Location);
void genOpenMPConstruct(AbstractConverter &, Fortran::lower::SymMap &,
semantics::SemanticsContext &, pft::Evaluation &,
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 6e93387fbff7cc..bb92fdce4ac56e 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -383,7 +383,8 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
// construct
mlir::OpBuilder::InsertPoint unstructuredSectionsIP =
firOpBuilder.saveInsertionPoint();
- firOpBuilder.setInsertionPointToStart(&op->getRegion(0).back());
+ mlir::Operation *lastOper = op->getRegion(0).back().getTerminator();
+ firOpBuilder.setInsertionPoint(lastOper);
lastPrivIP = firOpBuilder.saveInsertionPoint();
firOpBuilder.restoreInsertionPoint(unstructuredSectionsIP);
}
@@ -2133,15 +2134,6 @@ static mlir::Type getLoopVarType(Fortran::lower::AbstractConverter &converter,
return converter.getFirOpBuilder().getIntegerType(loopVarTypeSize);
}
-static void resetBeforeTerminator(fir::FirOpBuilder &firOpBuilder,
- mlir::Operation *storeOp,
- mlir::Block &block) {
- if (storeOp)
- firOpBuilder.setInsertionPointAfter(storeOp);
- else
- firOpBuilder.setInsertionPointToStart(&block);
-}
-
static mlir::Operation *
createAndSetPrivatizedLoopVar(Fortran::lower::AbstractConverter &converter,
mlir::Location loc, mlir::Value indexVal,
@@ -2183,11 +2175,43 @@ static void createBodyOfOp(
const llvm::SmallVector<const Fortran::semantics::Symbol *> &args = {},
bool outerCombined = false, DataSharingProcessor *dsp = nullptr) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+
+ auto insertMarker = [](fir::FirOpBuilder &builder) {
+ mlir::Value undef = builder.create<fir::UndefOp>(builder.getUnknownLoc(),
+ builder.getIndexType());
+ return undef.getDefiningOp();
+ };
+
+ // Find the block where the OMP terminator should go. In simple cases
+ // it is the single block in the operation's region. When the region
+ // is more complicated, especially with unstructured control flow, there
+ // may be multiple blocks, and some of them may have non-OMP terminators
+ // resulting from lowering of the code contained within the operation.
+ // By OpenMP rules, there should be a single exit point from the region:
+ // here exit means transfering control to the code following the operation.
+ // STOP statement is allowed and does not count as exit for the purpose of
+ // inserting terminators.
+ auto findExitBlock = [&](mlir::Region ®ion) -> mlir::Block * {
+ auto isTerminated = [](mlir::Block &block) -> bool {
+ if (block.empty())
+ return false;
+ return block.back().hasTrait<mlir::OpTrait::IsTerminator>();
+ };
+
+ mlir::Block *exit = nullptr;
+ for (auto &block : region) {
+ if (!isTerminated(block)) {
+ assert(exit == nullptr && "Multiple exit block in OpenMP region");
+ exit = █
+ }
+ }
+ return exit;
+ };
+
// 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.
// e.g. For loops the argument is the induction variable. And all further
// uses of the induction variable should use this mlir value.
- mlir::Operation *storeOp = nullptr;
if (args.size()) {
std::size_t loopVarTypeSize = 0;
for (const Fortran::semantics::Symbol *arg : args)
@@ -2198,20 +2222,20 @@ static void createBodyOfOp(
firOpBuilder.createBlock(&op.getRegion(), {}, tiv, locs);
// The argument is not currently in memory, so make a temporary for the
// argument, and store it there, then bind that location to the argument.
+ mlir::Operation *storeOp = nullptr;
for (auto [argIndex, argSymbol] : llvm::enumerate(args)) {
mlir::Value indexVal =
fir::getBase(op.getRegion().front().getArgument(argIndex));
storeOp =
createAndSetPrivatizedLoopVar(converter, loc, indexVal, argSymbol);
}
+ firOpBuilder.setInsertionPointAfter(storeOp);
} else {
firOpBuilder.createBlock(&op.getRegion());
}
- // Set the insert for the terminator operation to go at the end of the
- // block - this is either empty or the block with the stores above,
- // the end of the block works for both.
- mlir::Block &block = op.getRegion().back();
- firOpBuilder.setInsertionPointToEnd(&block);
+
+ // Mark the earliest insertion point.
+ mlir::Operation *marker = insertMarker(firOpBuilder);
// If it is an unstructured region and is not the outer region of a combined
// construct, create empty blocks for all evaluations.
@@ -2220,37 +2244,64 @@ static void createBodyOfOp(
mlir::omp::YieldOp>(
firOpBuilder, eval.getNestedEvaluations());
- // Insert the terminator.
- Fortran::lower::genOpenMPTerminator(firOpBuilder, op.getOperation(), loc);
- // Reset the insert point to before the terminator.
- resetBeforeTerminator(firOpBuilder, storeOp, block);
+ // Start with privatization, so that the lowering of the nested
+ // code will use the right symbols.
+ constexpr bool isLoop = std::is_same_v<Op, mlir::omp::WsLoopOp> ||
+ std::is_same_v<Op, mlir::omp::SimdLoopOp>;
+ bool privatize = clauses && !outerCombined;
- // Handle privatization. Do not privatize if this is the outer operation.
- if (clauses && !outerCombined) {
- constexpr bool isLoop = std::is_same_v<Op, mlir::omp::WsLoopOp> ||
- std::is_same_v<Op, mlir::omp::SimdLoopOp>;
+ firOpBuilder.setInsertionPoint(marker);
+ std::optional<DataSharingProcessor> tempDsp;
+ if (privatize) {
if (!dsp) {
- DataSharingProcessor proc(converter, *clauses, eval);
- proc.processStep1();
- proc.processStep2(op, isLoop);
- } else {
- if (isLoop && args.size() > 0)
- dsp->setLoopIV(converter.getSymbolAddress(*args[0]));
- dsp->processStep2(op, isLoop);
+ tempDsp.emplace(converter, *clauses, eval);
+ tempDsp->processStep1();
}
-
- if (storeOp)
- firOpBuilder.setInsertionPointAfter(storeOp);
}
if constexpr (std::is_same_v<Op, mlir::omp::ParallelOp>) {
threadPrivatizeVars(converter, eval);
- if (clauses)
+ if (clauses) {
+ firOpBuilder.setInsertionPoint(marker);
ClauseProcessor(converter, *clauses).processCopyin();
+ }
}
- if (genNested)
+ if (genNested) {
+ // genFIR(Evaluation&) tries to patch up unterminated blocks, causing
+ // a lot of trouble if the terminator generation is delayed past this
+ // point. Insert a temporary terminator here, then delete it.
+ firOpBuilder.setInsertionPointToEnd(&op.getRegion().back());
+ auto *temp = Fortran::lower::genOpenMPTerminator(firOpBuilder,
+ op.getOperation(), loc);
+ firOpBuilder.setInsertionPointAfter(marker);
genNestedEvaluations(converter, eval);
+ temp->erase();
+ }
+
+ if (auto *exitBlock = findExitBlock(op.getRegion())) {
+ firOpBuilder.setInsertionPointToEnd(exitBlock);
+ auto *term = Fortran::lower::genOpenMPTerminator(firOpBuilder,
+ op.getOperation(), loc);
+ // Only insert lastprivate code when there actually is an exit block.
+ // Such a block may not exist if the nested code produced an infinite
+ // loop (this may not make sense in production code, but a user could
+ // write that and we should handle it).
+ firOpBuilder.setInsertionPoint(term);
+ if (privatize) {
+ if (!dsp) {
+ assert(tempDsp.has_value());
+ tempDsp->processStep2(op, isLoop);
+ } else {
+ if (isLoop && args.size() > 0)
+ dsp->setLoopIV(converter.getSymbolAddress(*args[0]));
+ dsp->processStep2(op, isLoop);
+ }
+ }
+ }
+
+ firOpBuilder.setInsertionPointAfter(marker);
+ marker->erase();
}
static void genBodyOfTargetDataOp(
@@ -3664,14 +3715,14 @@ genOMP(Fortran::lower::AbstractConverter &converter,
// Public functions
//===----------------------------------------------------------------------===//
-void Fortran::lower::genOpenMPTerminator(fir::FirOpBuilder &builder,
- mlir::Operation *op,
- mlir::Location loc) {
+mlir::Operation *Fortran::lower::genOpenMPTerminator(fir::FirOpBuilder &builder,
+ mlir::Operation *op,
+ mlir::Location loc) {
if (mlir::isa<mlir::omp::WsLoopOp, mlir::omp::ReductionDeclareOp,
mlir::omp::AtomicUpdateOp, mlir::omp::SimdLoopOp>(op))
- builder.create<mlir::omp::YieldOp>(loc);
+ return builder.create<mlir::omp::YieldOp>(loc);
else
- builder.create<mlir::omp::TerminatorOp>(loc);
+ return builder.create<mlir::omp::TerminatorOp>(loc);
}
void Fortran::lower::genOpenMPConstruct(
diff --git a/flang/test/Lower/OpenMP/FIR/sections.f90 b/flang/test/Lower/OpenMP/FIR/sections.f90
index 87d34e58321be0..640ec34a05bc21 100644
--- a/flang/test/Lower/OpenMP/FIR/sections.f90
+++ b/flang/test/Lower/OpenMP/FIR/sections.f90
@@ -126,11 +126,11 @@ subroutine lastprivate()
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 %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: %[[const:.*]] = arith.constant 1 : i32
!CHECK: %[[result:.*]] = arith.addi %[[temp]], %[[const]] : i32
!CHECK: fir.store %[[result]] to %[[PRIVATE_X]] : !fir.ref<i32>
+!CHECK: %[[true:.*]] = arith.constant true
!CHECK: fir.if %[[true]] {
!CHECK: %[[temp:.*]] = fir.load %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: fir.store %[[temp]] to %[[X]] : !fir.ref<i32>
@@ -163,11 +163,11 @@ subroutine lastprivate()
!CHECK: %[[temp:.*]] = fir.load %[[X]] : !fir.ref<i32>
!CHECK: fir.store %[[temp]] to %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: omp.barrier
-!CHECK: %[[true:.*]] = arith.constant true
!CHECK: %[[temp:.*]] = fir.load %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: %[[const:.*]] = arith.constant 1 : i32
!CHECK: %[[result:.*]] = arith.addi %[[temp]], %[[const]] : i32
!CHECK: fir.store %[[result]] to %[[PRIVATE_X]] : !fir.ref<i32>
+!CHECK: %[[true:.*]] = arith.constant true
!CHECK: fir.if %true {
!CHECK: %[[temp:.*]] = fir.load %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: fir.store %[[temp]] to %[[X]] : !fir.ref<i32>
@@ -200,11 +200,11 @@ subroutine lastprivate()
!CHECK: %[[temp:.*]] = fir.load %[[X]] : !fir.ref<i32>
!CHECK: fir.store %[[temp]] to %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: omp.barrier
-!CHECK: %[[true:.*]] = arith.constant true
!CHECK: %[[temp:.*]] = fir.load %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: %[[const:.*]] = arith.constant 1 : i32
!CHECK: %[[result:.*]] = arith.addi %[[temp]], %[[const]] : i32
!CHECK: fir.store %[[result]] to %[[PRIVATE_X]] : !fir.ref<i32>
+!CHECK: %[[true:.*]] = arith.constant true
!CHECK: fir.if %true {
!CHECK: %[[temp:.*]] = fir.load %[[PRIVATE_X]] : !fir.ref<i32>
!CHECK: fir.store %[[temp]] to %[[X]] : !fir.ref<i32>
diff --git a/flang/test/Lower/OpenMP/infinite-loop-in-construct.f90 b/flang/test/Lower/OpenMP/infinite-loop-in-construct.f90
new file mode 100644
index 00000000000000..d44b440a72d90d
--- /dev/null
+++ b/flang/test/Lower/OpenMP/infinite-loop-in-construct.f90
@@ -0,0 +1,19 @@
+! RUN: bbc -fopenmp -o - %s | FileCheck %s
+
+! Check that this test can be lowered successfully.
+! See https://github.com/llvm/llvm-project/issues/74348
+
+! CHECK-LABEL: func.func @_QPsb
+! CHECK: omp.parallel
+subroutine sb(ninter, numnod)
+ integer :: ninter, numnod
+ integer, dimension(:), allocatable :: indx_nm
+
+ !$omp parallel
+ if (ninter>0) then
+ allocate(indx_nm(numnod))
+ endif
+ 220 continue
+ goto 220
+ !$omp end parallel
+end subroutine
diff --git a/flang/test/Lower/OpenMP/sections.f90 b/flang/test/Lower/OpenMP/sections.f90
index 16426d070d9a94..6bad688058d282 100644
--- a/flang/test/Lower/OpenMP/sections.f90
+++ b/flang/test/Lower/OpenMP/sections.f90
@@ -141,11 +141,11 @@ subroutine lastprivate()
!CHECK: omp.section {
!CHECK: %[[PRIVATE_X:.*]] = fir.alloca i32 {bindc_name = "x", pinned, uniq_name = "_QFlastprivateEx"}
!CHECK: %[[PRIVATE_X_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_X]] {uniq_name = "_QFlastprivateEx"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
-!CHECK: %[[TRUE:.*]] = arith.constant true
!CHECK: %[[TEMP:.*]] = fir.load %[[PRIVATE_X_DECL]]#0 : !fir.ref<i32>
!CHECK: %[[CONST:.*]] = arith.constant 1 : i32
!CHECK: %[[RESULT:.*]] = arith.addi %[[TEMP]], %[[CONST]] : i32
!CHECK: hlfir.assign %[[RESULT]] to %[[PRIVATE_X_DECL]]#0 : i32, !fir.ref<i32>
+!CHECK: %[[TRUE:.*]] = arith.constant true
!CHECK: fir.if %[[TRUE]] {
!CHECK: %[[TEMP1:.*]] = fir.load %[[PRIVATE_X_DECL]]#0 : !fir.ref<i32>
!CHECK: hlfir.assign %[[TEMP1]] to %[[X_DECL]]#0 temporary_lhs : i32, !fir.ref<i32>
@@ -180,11 +180,11 @@ subroutine lastprivate()
!CHECK: %[[TEMP:.*]] = fir.load %[[X_DECL]]#0 : !fir.ref<i32>
!CHECK: hlfir.assign %[[TEMP]] to %[[PRIVATE_X_DECL]]#0 temporary_lhs : i32, !fir.ref<i32>
!CHECK: omp.barrier
-!CHECK: %[[TRUE:.*]] = arith.constant true
!CHECK: %[[TEMP:.*]] = fir.load %[[PRIVATE_X_DECL]]#0 : !fir.ref<i32>
!CHECK: %[[CONST:.*]] = arith.constant 1 : i32
!CHECK: %[[RESULT:.*]] = arith.addi %[[TEMP]], %[[CONST]] : i32
!CHECK: hlfir.assign %[[RESULT]] to %[[PRIVATE_X_DECL]]#0 : i32, !fir.ref<i32>
+!CHECK: %[[TRUE:.*]] = arith.constant true
!CHECK: fir.if %[[TRUE]] {
!CHECK: %[[TEMP:.*]] = fir.load %[[PRIVATE_X_DECL]]#0 : !fir.ref<i32>
!CHECK: hlfir.assign %[[TEMP]] to %[[X_DECL]]#0 temporary_lhs : i32, !fir.ref<i32>
@@ -219,11 +219,11 @@ subroutine lastprivate()
!CHECK: %[[TEMP:.*]] = fir.load %[[X_DECL]]#0 : !fir.ref<i32>
!CHECK: hlfir.assign %[[TEMP]] to %[[PRIVATE_X_DECL]]#0 temporary_lhs : i32, !fir.ref<i32>
!CHECK: omp.barrier
-!CHECK: %[[TRUE:.*]] = arith.constant true
!CHECK: %[[TEMP:.*]] = fir.load %[[PRIVATE_X_DECL]]#0 : !fir.ref<i32>
!CHECK: %[[CONST:.*]] = arith.constant 1 : i32
!CHECK: %[[RESULT:.*]] = arith.addi %[[TEMP]], %[[CONST]] : i32
!CHECK: hlfir.assign %[[RESULT]] to %[[PRIVATE_X_DECL]]#0 : i32, !fir.ref<i32>
+!CHECK: %[[TRUE:.*]] = arith.constant true
!CHECK: fir.if %[[TRUE]] {
!CHECK: %[[TEMP:.*]] = fir.load %[[PRIVATE_X_DECL]]#0 : !fir.ref<i32>
!CHECK: hlfir.assign %[[TEMP]] to %[[X_DECL]]#0 temporary_lhs : i32, !fir.ref<i32>
>From cb3f2670a1b66dbb2e2d7789ea4ed95e7964ddef Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 15 Jan 2024 12:10:55 -0600
Subject: [PATCH 06/11] Add parameter description for genNested
---
flang/lib/Lower/OpenMP.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 6994aa8559b648..a8c3034329465c 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -2172,6 +2172,7 @@ createAndSetPrivatizedLoopVar(Fortran::lower::AbstractConverter &converter,
/// \param [inout] converter - converter to use for the clauses.
/// \param [in] loc - location in source code.
/// \param [in] eval - current PFT node/evaluation.
+/// \param [in] genNested - whether to generate FIR for nested evaluations
/// \oaran [in] clauses - list of clauses to process.
/// \param [in] args - block arguments (induction variable[s]) for the
//// region.
>From a1b59bfe0783fb1ebf90344a1261b4efe073aafe Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 15 Jan 2024 13:00:03 -0600
Subject: [PATCH 07/11] Add more detailed checks to testcase
---
.../test/Lower/OpenMP/infinite-loop-in-construct.f90 | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/flang/test/Lower/OpenMP/infinite-loop-in-construct.f90 b/flang/test/Lower/OpenMP/infinite-loop-in-construct.f90
index d44b440a72d90d..16b400a2318609 100644
--- a/flang/test/Lower/OpenMP/infinite-loop-in-construct.f90
+++ b/flang/test/Lower/OpenMP/infinite-loop-in-construct.f90
@@ -3,8 +3,15 @@
! Check that this test can be lowered successfully.
! See https://github.com/llvm/llvm-project/issues/74348
-! CHECK-LABEL: func.func @_QPsb
-! CHECK: omp.parallel
+! CHECK-LABEL: func.func @_QPsb
+! CHECK: omp.parallel
+! CHECK: cf.cond_br %{{[0-9]+}}, ^bb1, ^bb2
+! CHECK-NEXT: ^bb1: // pred: ^bb0
+! CHECK: cf.br ^bb2
+! CHECK-NEXT: ^bb2: // 3 preds: ^bb0, ^bb1, ^bb2
+! CHECK-NEXT: cf.br ^bb2
+! CHECK-NEXT: }
+
subroutine sb(ninter, numnod)
integer :: ninter, numnod
integer, dimension(:), allocatable :: indx_nm
>From 8ead6c5ae0df086c04ff6738fcec72525bd0f6d0 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 15 Jan 2024 13:10:54 -0600
Subject: [PATCH 08/11] Add testcase from
https://github.com/llvm/llvm-project/pull/77329
---
.../test/Lower/OpenMP/wsloop-unstructured.f90 | 61 +++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 flang/test/Lower/OpenMP/wsloop-unstructured.f90
diff --git a/flang/test/Lower/OpenMP/wsloop-unstructured.f90 b/flang/test/Lower/OpenMP/wsloop-unstructured.f90
new file mode 100644
index 00000000000000..7fe63a1fe607c2
--- /dev/null
+++ b/flang/test/Lower/OpenMP/wsloop-unstructured.f90
@@ -0,0 +1,61 @@
+! RUN: bbc -emit-hlfir -fopenmp -o - %s | FileCheck %s
+
+subroutine sub(imax, jmax, x, y)
+ integer, intent(in) :: imax, jmax
+ real, intent(in), dimension(1:imax, 1:jmax) :: x, y
+
+ integer :: i, j, ii
+
+ ! collapse(2) is needed to reproduce the issue
+ !$omp parallel do collapse(2)
+ do j = 1, jmax
+ do i = 1, imax
+ do ii = 1, imax ! note that this loop is not collapsed
+ if (x(i,j) < y(ii,j)) then
+ ! exit needed to force unstructured control flow
+ exit
+ endif
+ enddo
+ enddo
+ enddo
+end subroutine sub
+
+! this is testing that we don't crash generating code for this: in particular
+! that all blocks are terminated
+
+! CHECK-LABEL: func.func @_QPsub(
+! CHECK-SAME: %[[VAL_0:.*]]: !fir.ref<i32> {fir.bindc_name = "imax"},
+! CHECK-SAME: %[[VAL_1:.*]]: !fir.ref<i32> {fir.bindc_name = "jmax"},
+! CHECK-SAME: %[[VAL_2:.*]]: !fir.ref<!fir.array<?x?xf32>> {fir.bindc_name = "x"},
+! CHECK-SAME: %[[VAL_3:.*]]: !fir.ref<!fir.array<?x?xf32>> {fir.bindc_name = "y"}) {
+! [...]
+! CHECK: omp.wsloop for (%[[VAL_53:.*]], %[[VAL_54:.*]]) : i32 = ({{.*}}) to ({{.*}}) inclusive step ({{.*}}) {
+! [...]
+! CHECK: cf.br ^bb1
+! CHECK: ^bb1:
+! CHECK: cf.br ^bb2
+! CHECK: ^bb2:
+! [...]
+! CHECK: cf.br ^bb3
+! CHECK: ^bb3:
+! [...]
+! CHECK: %[[VAL_63:.*]] = arith.cmpi sgt, %{{.*}}, %{{.*}} : i32
+! CHECK: cf.cond_br %[[VAL_63]], ^bb4, ^bb7
+! CHECK: ^bb4:
+! [...]
+! CHECK: %[[VAL_76:.*]] = arith.cmpf olt, %{{.*}}, %{{.*}} fastmath<contract> : f32
+! CHECK: cf.cond_br %[[VAL_76]], ^bb5, ^bb6
+! CHECK: ^bb5:
+! CHECK: cf.br ^bb7
+! CHECK: ^bb6:
+! [...]
+! CHECK: cf.br ^bb3
+! CHECK: ^bb7:
+! CHECK: omp.yield
+! CHECK: }
+! CHECK: omp.terminator
+! CHECK: }
+! CHECK: cf.br ^bb1
+! CHECK: ^bb1:
+! CHECK: return
+! CHECK: }
>From 1207d602d9f959cf095540d440099fd5a5efc4a8 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 18 Jan 2024 14:38:00 -0600
Subject: [PATCH 10/11] Replace assertion with constructing unique block
If there are multiple exiting blocks, create a new empty block,
and insert branches into it from all the exiting blocks. This
will create a unique exiting block for inserting OMP terminator,
and lastprivate processing.
---
flang/lib/Lower/OpenMP.cpp | 65 ++++++++++++++++++++++----------------
1 file changed, 38 insertions(+), 27 deletions(-)
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 71bf2659fe8255..f2a49b4f4d1bb0 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -27,6 +27,7 @@
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/openmp-directive-sets.h"
#include "flang/Semantics/tools.h"
+#include "mlir/Dialect/ControlFlow/IR/ControlFlowOps.h"
#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
#include "mlir/Dialect/SCF/IR/SCF.h"
#include "mlir/Transforms/RegionUtils.h"
@@ -2185,32 +2186,6 @@ static void createBodyOfOp(
return undef.getDefiningOp();
};
- // Find the block where the OMP terminator should go. In simple cases
- // it is the single block in the operation's region. When the region
- // is more complicated, especially with unstructured control flow, there
- // may be multiple blocks, and some of them may have non-OMP terminators
- // resulting from lowering of the code contained within the operation.
- // By OpenMP rules, there should be a single exit point from the region:
- // here exit means transfering control to the code following the operation.
- // STOP statement is allowed and does not count as exit for the purpose of
- // inserting terminators.
- auto findExitBlock = [&](mlir::Region ®ion) -> mlir::Block * {
- auto isTerminated = [](mlir::Block &block) -> bool {
- if (block.empty())
- return false;
- return block.back().hasTrait<mlir::OpTrait::IsTerminator>();
- };
-
- mlir::Block *exit = nullptr;
- for (auto &block : region) {
- if (!isTerminated(block)) {
- assert(exit == nullptr && "Multiple exit block in OpenMP region");
- exit = █
- }
- }
- return exit;
- };
-
// 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.
// e.g. For loops the argument is the induction variable. And all further
@@ -2282,7 +2257,43 @@ static void createBodyOfOp(
temp->erase();
}
- if (auto *exitBlock = findExitBlock(op.getRegion())) {
+ // Get or create a unique exiting block from the given region, or
+ // return nullptr if there is no exiting block.
+ auto getUniqueExit = [&](mlir::Region ®ion) -> mlir::Block * {
+ // Find the blocks where the OMP terminator should go. In simple cases
+ // it is the single block in the operation's region. When the region
+ // is more complicated, especially with unstructured control flow, there
+ // may be multiple blocks, and some of them may have non-OMP terminators
+ // resulting from lowering of the code contained within the operation.
+ // All the remaining blocks are potential exit points from the op's region.
+ //
+ // Explicit control flow cannot exit any OpenMP region (other than via
+ // STOP), and that is enforced by semantic checks prior to lowering. STOP
+ // statements are lowered to a function call.
+
+ // Collect unterminated blocks.
+ llvm::SmallVector<mlir::Block *> exits;
+ for (mlir::Block &b : region) {
+ if (b.empty() || !b.back().hasTrait<mlir::OpTrait::IsTerminator>())
+ exits.push_back(&b);
+ }
+
+ if (exits.empty())
+ return nullptr;
+ // If there already is a unique exiting block, do not create another one.
+ // Additionally, some ops (e.g. omp.sections) require onlt 1 block in
+ // its region.
+ if (exits.size() == 1)
+ return exits[0];
+ mlir::Block *exit = firOpBuilder.createBlock(®ion);
+ for (mlir::Block *b : exits) {
+ firOpBuilder.setInsertionPointToEnd(b);
+ firOpBuilder.create<mlir::cf::BranchOp>(loc, exit);
+ }
+ return exit;
+ };
+
+ if (auto *exitBlock = getUniqueExit(op.getRegion())) {
firOpBuilder.setInsertionPointToEnd(exitBlock);
auto *term = Fortran::lower::genOpenMPTerminator(firOpBuilder,
op.getOperation(), loc);
>From bb6795e689a77113b5a0398968cc05bed70a7633 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Fri, 19 Jan 2024 10:58:53 -0600
Subject: [PATCH 11/11] Update flang/lib/Lower/OpenMP.cpp
Fix typo in comment
Co-authored-by: Kiran Chandramohan <kiran.chandramohan at arm.com>
---
flang/lib/Lower/OpenMP.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index f2a49b4f4d1bb0..6a40bd54a2f3b0 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -2281,7 +2281,7 @@ static void createBodyOfOp(
if (exits.empty())
return nullptr;
// If there already is a unique exiting block, do not create another one.
- // Additionally, some ops (e.g. omp.sections) require onlt 1 block in
+ // Additionally, some ops (e.g. omp.sections) require only 1 block in
// its region.
if (exits.size() == 1)
return exits[0];
More information about the flang-commits
mailing list