New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[flang][OpenMP] Convert DataSharingProcessor to omp::Clause #81629
Conversation
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-openmp Author: Krzysztof Parzyszek (kparzysz) Changes[Clause representation 6/6] Full diff: https://github.com/llvm/llvm-project/pull/81629.diff 1 Files Affected:
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 8dcd0708e6245..88402828053e6 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -1277,14 +1277,15 @@ class DataSharingProcessor {
llvm::SetVector<const Fortran::semantics::Symbol *> symbolsInNestedRegions;
llvm::SetVector<const Fortran::semantics::Symbol *> symbolsInParentRegions;
Fortran::lower::AbstractConverter &converter;
+ Fortran::semantics::SemanticsContext &semaCtx;
fir::FirOpBuilder &firOpBuilder;
- const Fortran::parser::OmpClauseList &opClauseList;
+ omp::List<omp::Clause> clauses;
Fortran::lower::pft::Evaluation &eval;
bool needBarrier();
void collectSymbols(Fortran::semantics::Symbol::Flag flag);
void collectOmpObjectListSymbol(
- const Fortran::parser::OmpObjectList &ompObjectList,
+ const omp::ObjectList &objects,
llvm::SetVector<const Fortran::semantics::Symbol *> &symbolSet);
void collectSymbolsForPrivatization();
void insertBarrier();
@@ -1301,11 +1302,12 @@ class DataSharingProcessor {
public:
DataSharingProcessor(Fortran::lower::AbstractConverter &converter,
+ Fortran::semantics::SemanticsContext &semaCtx,
const Fortran::parser::OmpClauseList &opClauseList,
Fortran::lower::pft::Evaluation &eval)
- : hasLastPrivateOp(false), converter(converter),
- firOpBuilder(converter.getFirOpBuilder()), opClauseList(opClauseList),
- eval(eval) {}
+ : hasLastPrivateOp(false), converter(converter), semaCtx(semaCtx),
+ firOpBuilder(converter.getFirOpBuilder()),
+ clauses(omp::makeList(opClauseList, semaCtx)), eval(eval) {}
// Privatisation is split into two steps.
// Step1 performs cloning of all privatisation clauses and copying for
// firstprivates. Step1 is performed at the place where process/processStep1
@@ -1383,30 +1385,28 @@ void DataSharingProcessor::copyLastPrivateSymbol(
}
void DataSharingProcessor::collectOmpObjectListSymbol(
- const Fortran::parser::OmpObjectList &ompObjectList,
+ const omp::ObjectList &objects,
llvm::SetVector<const Fortran::semantics::Symbol *> &symbolSet) {
- for (const Fortran::parser::OmpObject &ompObject : ompObjectList.v) {
- Fortran::semantics::Symbol *sym = getOmpObjectSymbol(ompObject);
+ for (const omp::Object &object : objects) {
+ Fortran::semantics::Symbol *sym = object.sym;
symbolSet.insert(sym);
}
}
void DataSharingProcessor::collectSymbolsForPrivatization() {
bool hasCollapse = false;
- for (const Fortran::parser::OmpClause &clause : opClauseList.v) {
+ for (const omp::Clause &clause : clauses) {
if (const auto &privateClause =
- std::get_if<Fortran::parser::OmpClause::Private>(&clause.u)) {
+ std::get_if<omp::clause::Private>(&clause.u)) {
collectOmpObjectListSymbol(privateClause->v, privatizedSymbols);
} else if (const auto &firstPrivateClause =
- std::get_if<Fortran::parser::OmpClause::Firstprivate>(
- &clause.u)) {
+ std::get_if<omp::clause::Firstprivate>(&clause.u)) {
collectOmpObjectListSymbol(firstPrivateClause->v, privatizedSymbols);
} else if (const auto &lastPrivateClause =
- std::get_if<Fortran::parser::OmpClause::Lastprivate>(
- &clause.u)) {
+ std::get_if<omp::clause::Lastprivate>(&clause.u)) {
collectOmpObjectListSymbol(lastPrivateClause->v, privatizedSymbols);
hasLastPrivateOp = true;
- } else if (std::get_if<Fortran::parser::OmpClause::Collapse>(&clause.u)) {
+ } else if (std::get_if<omp::clause::Collapse>(&clause.u)) {
hasCollapse = true;
}
}
@@ -1439,138 +1439,135 @@ void DataSharingProcessor::insertBarrier() {
void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
bool cmpCreated = false;
mlir::OpBuilder::InsertPoint localInsPt = firOpBuilder.saveInsertionPoint();
- for (const Fortran::parser::OmpClause &clause : opClauseList.v) {
- if (std::get_if<Fortran::parser::OmpClause::Lastprivate>(&clause.u)) {
- // TODO: Add lastprivate support for simd construct
- if (mlir::isa<mlir::omp::SectionOp>(op)) {
- if (&eval == &eval.parentConstruct->getLastNestedEvaluation()) {
- // For `omp.sections`, lastprivatized variables occur in
- // lexically final `omp.section` operation. The following FIR
- // shall be generated for the same:
- //
- // omp.sections lastprivate(...) {
- // omp.section {...}
- // omp.section {...}
- // omp.section {
- // fir.allocate for `private`/`firstprivate`
- // <More operations here>
- // fir.if %true {
- // ^%lpv_update_blk
- // }
- // }
- // }
- //
- // To keep code consistency while handling privatization
- // through this control flow, add a `fir.if` operation
- // that always evaluates to true, in order to create
- // a dedicated sub-region in `omp.section` where
- // lastprivate FIR can reside. Later canonicalizations
- // will optimize away this operation.
- if (!eval.lowerAsUnstructured()) {
- auto ifOp = firOpBuilder.create<fir::IfOp>(
- op->getLoc(),
- firOpBuilder.createIntegerConstant(
- op->getLoc(), firOpBuilder.getIntegerType(1), 0x1),
- /*else*/ false);
- firOpBuilder.setInsertionPointToStart(
- &ifOp.getThenRegion().front());
-
- const Fortran::parser::OpenMPConstruct *parentOmpConstruct =
- eval.parentConstruct->getIf<Fortran::parser::OpenMPConstruct>();
- assert(parentOmpConstruct &&
- "Expected a valid enclosing OpenMP construct");
- const Fortran::parser::OpenMPSectionsConstruct *sectionsConstruct =
- std::get_if<Fortran::parser::OpenMPSectionsConstruct>(
- &parentOmpConstruct->u);
- assert(sectionsConstruct &&
- "Expected an enclosing omp.sections construct");
- const Fortran::parser::OmpClauseList §ionsEndClauseList =
- std::get<Fortran::parser::OmpClauseList>(
- std::get<Fortran::parser::OmpEndSectionsDirective>(
- sectionsConstruct->t)
- .t);
- for (const Fortran::parser::OmpClause &otherClause :
- sectionsEndClauseList.v)
- if (std::get_if<Fortran::parser::OmpClause::Nowait>(
- &otherClause.u))
- // Emit implicit barrier to synchronize threads and avoid data
- // races on post-update of lastprivate variables when `nowait`
- // clause is present.
- firOpBuilder.create<mlir::omp::BarrierOp>(
- converter.getCurrentLocation());
- firOpBuilder.setInsertionPointToStart(
- &ifOp.getThenRegion().front());
- lastPrivIP = firOpBuilder.saveInsertionPoint();
- firOpBuilder.setInsertionPoint(ifOp);
- insPt = firOpBuilder.saveInsertionPoint();
- } else {
- // Lastprivate operation is inserted at the end
- // of the lexically last section in the sections
- // construct
- mlir::OpBuilder::InsertPoint unstructuredSectionsIP =
- firOpBuilder.saveInsertionPoint();
- mlir::Operation *lastOper = op->getRegion(0).back().getTerminator();
- firOpBuilder.setInsertionPoint(lastOper);
- lastPrivIP = firOpBuilder.saveInsertionPoint();
- firOpBuilder.restoreInsertionPoint(unstructuredSectionsIP);
- }
- }
- } else if (mlir::isa<mlir::omp::WsLoopOp>(op)) {
- // Update the original variable just before exiting the worksharing
- // loop. Conversion as follows:
+ for (const omp::Clause &clause : clauses) {
+ if (clause.id != llvm::omp::OMPC_lastprivate)
+ continue;
+ // TODO: Add lastprivate support for simd construct
+ if (mlir::isa<mlir::omp::SectionOp>(op)) {
+ if (&eval == &eval.parentConstruct->getLastNestedEvaluation()) {
+ // For `omp.sections`, lastprivatized variables occur in
+ // lexically final `omp.section` operation. The following FIR
+ // shall be generated for the same:
//
- // omp.wsloop {
- // omp.wsloop { ...
- // ... store
- // store ===> %v = arith.addi %iv, %step
- // omp.yield %cmp = %step < 0 ? %v < %ub : %v > %ub
- // } fir.if %cmp {
- // fir.store %v to %loopIV
- // ^%lpv_update_blk:
- // }
- // omp.yield
- // }
+ // omp.sections lastprivate(...) {
+ // omp.section {...}
+ // omp.section {...}
+ // omp.section {
+ // fir.allocate for `private`/`firstprivate`
+ // <More operations here>
+ // fir.if %true {
+ // ^%lpv_update_blk
+ // }
+ // }
+ // }
//
-
- // Only generate the compare once in presence of multiple LastPrivate
- // clauses.
- if (cmpCreated)
- continue;
- cmpCreated = true;
-
- mlir::Location loc = op->getLoc();
- mlir::Operation *lastOper = op->getRegion(0).back().getTerminator();
- firOpBuilder.setInsertionPoint(lastOper);
-
- mlir::Value iv = op->getRegion(0).front().getArguments()[0];
- mlir::Value ub =
- mlir::dyn_cast<mlir::omp::WsLoopOp>(op).getUpperBound()[0];
- mlir::Value step = mlir::dyn_cast<mlir::omp::WsLoopOp>(op).getStep()[0];
-
- // v = iv + step
- // cmp = step < 0 ? v < ub : v > ub
- mlir::Value v = firOpBuilder.create<mlir::arith::AddIOp>(loc, iv, step);
- mlir::Value zero =
- firOpBuilder.createIntegerConstant(loc, step.getType(), 0);
- mlir::Value negativeStep = firOpBuilder.create<mlir::arith::CmpIOp>(
- loc, mlir::arith::CmpIPredicate::slt, step, zero);
- mlir::Value vLT = firOpBuilder.create<mlir::arith::CmpIOp>(
- loc, mlir::arith::CmpIPredicate::slt, v, ub);
- mlir::Value vGT = firOpBuilder.create<mlir::arith::CmpIOp>(
- loc, mlir::arith::CmpIPredicate::sgt, v, ub);
- mlir::Value cmpOp = firOpBuilder.create<mlir::arith::SelectOp>(
- loc, negativeStep, vLT, vGT);
-
- auto ifOp = firOpBuilder.create<fir::IfOp>(loc, cmpOp, /*else*/ false);
- firOpBuilder.setInsertionPointToStart(&ifOp.getThenRegion().front());
- assert(loopIV && "loopIV was not set");
- firOpBuilder.create<fir::StoreOp>(op->getLoc(), v, loopIV);
- lastPrivIP = firOpBuilder.saveInsertionPoint();
- } else {
- TODO(converter.getCurrentLocation(),
- "lastprivate clause in constructs other than "
- "simd/worksharing-loop");
+ // To keep code consistency while handling privatization
+ // through this control flow, add a `fir.if` operation
+ // that always evaluates to true, in order to create
+ // a dedicated sub-region in `omp.section` where
+ // lastprivate FIR can reside. Later canonicalizations
+ // will optimize away this operation.
+ if (!eval.lowerAsUnstructured()) {
+ auto ifOp = firOpBuilder.create<fir::IfOp>(
+ op->getLoc(),
+ firOpBuilder.createIntegerConstant(
+ op->getLoc(), firOpBuilder.getIntegerType(1), 0x1),
+ /*else*/ false);
+ firOpBuilder.setInsertionPointToStart(&ifOp.getThenRegion().front());
+
+ const Fortran::parser::OpenMPConstruct *parentOmpConstruct =
+ eval.parentConstruct->getIf<Fortran::parser::OpenMPConstruct>();
+ assert(parentOmpConstruct &&
+ "Expected a valid enclosing OpenMP construct");
+ const Fortran::parser::OpenMPSectionsConstruct *sectionsConstruct =
+ std::get_if<Fortran::parser::OpenMPSectionsConstruct>(
+ &parentOmpConstruct->u);
+ assert(sectionsConstruct &&
+ "Expected an enclosing omp.sections construct");
+ const Fortran::parser::OmpClauseList §ionsEndClauseList =
+ std::get<Fortran::parser::OmpClauseList>(
+ std::get<Fortran::parser::OmpEndSectionsDirective>(
+ sectionsConstruct->t)
+ .t);
+ for (const Fortran::parser::OmpClause &otherClause :
+ sectionsEndClauseList.v)
+ if (std::get_if<Fortran::parser::OmpClause::Nowait>(&otherClause.u))
+ // Emit implicit barrier to synchronize threads and avoid data
+ // races on post-update of lastprivate variables when `nowait`
+ // clause is present.
+ firOpBuilder.create<mlir::omp::BarrierOp>(
+ converter.getCurrentLocation());
+ firOpBuilder.setInsertionPointToStart(&ifOp.getThenRegion().front());
+ lastPrivIP = firOpBuilder.saveInsertionPoint();
+ firOpBuilder.setInsertionPoint(ifOp);
+ insPt = firOpBuilder.saveInsertionPoint();
+ } else {
+ // Lastprivate operation is inserted at the end
+ // of the lexically last section in the sections
+ // construct
+ mlir::OpBuilder::InsertPoint unstructuredSectionsIP =
+ firOpBuilder.saveInsertionPoint();
+ mlir::Operation *lastOper = op->getRegion(0).back().getTerminator();
+ firOpBuilder.setInsertionPoint(lastOper);
+ lastPrivIP = firOpBuilder.saveInsertionPoint();
+ firOpBuilder.restoreInsertionPoint(unstructuredSectionsIP);
+ }
}
+ } else if (mlir::isa<mlir::omp::WsLoopOp>(op)) {
+ // Update the original variable just before exiting the worksharing
+ // loop. Conversion as follows:
+ //
+ // omp.wsloop {
+ // omp.wsloop { ...
+ // ... store
+ // store ===> %v = arith.addi %iv, %step
+ // omp.yield %cmp = %step < 0 ? %v < %ub : %v > %ub
+ // } fir.if %cmp {
+ // fir.store %v to %loopIV
+ // ^%lpv_update_blk:
+ // }
+ // omp.yield
+ // }
+ //
+
+ // Only generate the compare once in presence of multiple LastPrivate
+ // clauses.
+ if (cmpCreated)
+ continue;
+ cmpCreated = true;
+
+ mlir::Location loc = op->getLoc();
+ mlir::Operation *lastOper = op->getRegion(0).back().getTerminator();
+ firOpBuilder.setInsertionPoint(lastOper);
+
+ mlir::Value iv = op->getRegion(0).front().getArguments()[0];
+ mlir::Value ub =
+ mlir::dyn_cast<mlir::omp::WsLoopOp>(op).getUpperBound()[0];
+ mlir::Value step = mlir::dyn_cast<mlir::omp::WsLoopOp>(op).getStep()[0];
+
+ // v = iv + step
+ // cmp = step < 0 ? v < ub : v > ub
+ mlir::Value v = firOpBuilder.create<mlir::arith::AddIOp>(loc, iv, step);
+ mlir::Value zero =
+ firOpBuilder.createIntegerConstant(loc, step.getType(), 0);
+ mlir::Value negativeStep = firOpBuilder.create<mlir::arith::CmpIOp>(
+ loc, mlir::arith::CmpIPredicate::slt, step, zero);
+ mlir::Value vLT = firOpBuilder.create<mlir::arith::CmpIOp>(
+ loc, mlir::arith::CmpIPredicate::slt, v, ub);
+ mlir::Value vGT = firOpBuilder.create<mlir::arith::CmpIOp>(
+ loc, mlir::arith::CmpIPredicate::sgt, v, ub);
+ mlir::Value cmpOp = firOpBuilder.create<mlir::arith::SelectOp>(
+ loc, negativeStep, vLT, vGT);
+
+ auto ifOp = firOpBuilder.create<fir::IfOp>(loc, cmpOp, /*else*/ false);
+ firOpBuilder.setInsertionPointToStart(&ifOp.getThenRegion().front());
+ assert(loopIV && "loopIV was not set");
+ firOpBuilder.create<fir::StoreOp>(op->getLoc(), v, loopIV);
+ lastPrivIP = firOpBuilder.saveInsertionPoint();
+ } else {
+ TODO(converter.getCurrentLocation(),
+ "lastprivate clause in constructs other than "
+ "simd/worksharing-loop");
}
}
firOpBuilder.restoreInsertionPoint(localInsPt);
@@ -1594,14 +1591,12 @@ void DataSharingProcessor::collectSymbols(
}
void DataSharingProcessor::collectDefaultSymbols() {
- for (const Fortran::parser::OmpClause &clause : opClauseList.v) {
- if (const auto &defaultClause =
- std::get_if<Fortran::parser::OmpClause::Default>(&clause.u)) {
- if (defaultClause->v.v ==
- Fortran::parser::OmpDefaultClause::Type::Private)
+ for (const omp::Clause &clause : clauses) {
+ if (const auto *defaultClause =
+ std::get_if<omp::clause::Default>(&clause.u)) {
+ if (defaultClause->v == omp::clause::Default::Type::Private)
collectSymbols(Fortran::semantics::Symbol::Flag::OmpPrivate);
- else if (defaultClause->v.v ==
- Fortran::parser::OmpDefaultClause::Type::Firstprivate)
+ else if (defaultClause->v == omp::clause::Default::Type::Firstprivate)
collectSymbols(Fortran::semantics::Symbol::Flag::OmpFirstPrivate);
}
}
@@ -3446,7 +3441,7 @@ static void createBodyOfOp(Op &op, OpWithBodyGenInfo &info) {
std::optional<DataSharingProcessor> tempDsp;
if (privatize) {
if (!info.dsp) {
- tempDsp.emplace(info.converter, *info.clauses, info.eval);
+ tempDsp.emplace(info.converter, info.semaCtx, *info.clauses, info.eval);
tempDsp->processStep1();
}
}
@@ -4392,7 +4387,7 @@ createSimdLoop(Fortran::lower::AbstractConverter &converter,
const Fortran::parser::OmpClauseList &loopOpClauseList,
mlir::Location loc) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- DataSharingProcessor dsp(converter, loopOpClauseList, eval);
+ DataSharingProcessor dsp(converter, semaCtx, loopOpClauseList, eval);
dsp.processStep1();
Fortran::lower::StatementContext stmtCtx;
@@ -4449,7 +4444,7 @@ static void createWsLoop(Fortran::lower::AbstractConverter &converter,
const Fortran::parser::OmpClauseList *endClauseList,
mlir::Location loc) {
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- DataSharingProcessor dsp(converter, beginClauseList, eval);
+ DataSharingProcessor dsp(converter, semaCtx, beginClauseList, eval);
dsp.processStep1();
Fortran::lower::StatementContext stmtCtx;
|
c5adb1d
to
61d3ad3
Compare
2e00886
to
1299d51
Compare
61d3ad3
to
b4351a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just small comments.
ddcdd20
to
7e785c4
Compare
b4351a1
to
158901f
Compare
@@ -135,138 +133,135 @@ void DataSharingProcessor::insertBarrier() { | |||
void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) { | |||
bool cmpCreated = false; | |||
mlir::OpBuilder::InsertPoint localInsPt = firOpBuilder.saveInsertionPoint(); | |||
for (const Fortran::parser::OmpClause &clause : opClauseList.v) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there code changes here or is it only the replacement of parser::OmpClause
with omp::Clause
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code functionality is unchanged. One reason for the seemingly large differences in the diff is that I replaced the if (get_if<LastPrivate>)
with if (!get_if<Lastprivate>) continue;
to reduce indentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LG.
[Clause representation 6/6] This can be committed out of order, because the prerequitites have already been merged.
158901f
to
0df6d56
Compare
[Clause representation 6/6]