[flang][OpenMP] Try to unify induction var privatization for OMP regions. #91116
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,9 +29,10 @@ void DataSharingProcessor::processStep1( | |
| collectSymbolsForPrivatization(); | ||
| collectDefaultSymbols(); | ||
| collectImplicitSymbols(); | ||
| collectPreDeterminedSymbols(); | ||
|
|
||
| privatize(clauseOps, privateSyms); | ||
|
|
||
| insertBarrier(); | ||
| } | ||
|
|
||
|
|
@@ -57,7 +58,7 @@ void DataSharingProcessor::processStep2(mlir::Operation *op, bool isLoop) { | |
| } | ||
|
|
||
| void DataSharingProcessor::insertDeallocs() { | ||
| for (const Fortran::semantics::Symbol *sym : allPrivatizedSymbols) | ||
| if (Fortran::semantics::IsAllocatable(sym->GetUltimate())) { | ||
| if (!useDelayedPrivatization) { | ||
| converter.createHostAssociateVarCloneDealloc(*sym); | ||
|
|
@@ -92,10 +93,6 @@ void DataSharingProcessor::insertDeallocs() { | |
| } | ||
|
|
||
| void DataSharingProcessor::cloneSymbol(const Fortran::semantics::Symbol *sym) { | ||
| bool success = converter.createHostAssociateVarClone(*sym); | ||
| (void)success; | ||
| assert(success && "Privatization failed due to existing binding"); | ||
|
|
@@ -127,20 +124,24 @@ void DataSharingProcessor::collectSymbolsForPrivatization() { | |
| for (const omp::Clause &clause : clauses) { | ||
| if (const auto &privateClause = | ||
| std::get_if<omp::clause::Private>(&clause.u)) { | ||
| collectOmpObjectListSymbol(privateClause->v, explicitlyPrivatizedSymbols); | ||
| } else if (const auto &firstPrivateClause = | ||
| std::get_if<omp::clause::Firstprivate>(&clause.u)) { | ||
| collectOmpObjectListSymbol(firstPrivateClause->v, | ||
| explicitlyPrivatizedSymbols); | ||
| } else if (const auto &lastPrivateClause = | ||
| std::get_if<omp::clause::Lastprivate>(&clause.u)) { | ||
| const ObjectList &objects = std::get<ObjectList>(lastPrivateClause->t); | ||
| collectOmpObjectListSymbol(objects, explicitlyPrivatizedSymbols); | ||
| hasLastPrivateOp = true; | ||
| } else if (std::get_if<omp::clause::Collapse>(&clause.u)) { | ||
| hasCollapse = true; | ||
| } | ||
| } | ||
|
|
||
| for (auto *sym : explicitlyPrivatizedSymbols) | ||
| allPrivatizedSymbols.insert(sym); | ||
|
|
||
| if (hasCollapse && hasLastPrivateOp) | ||
| TODO(converter.getCurrentLocation(), "Collapse clause with lastprivate"); | ||
| } | ||
|
|
@@ -150,7 +151,7 @@ bool DataSharingProcessor::needBarrier() { | |
| // initialization of firstprivate variables and post-update of lastprivate | ||
| // variables. | ||
| // Emit implicit barrier for linear clause. Maybe on somewhere else. | ||
| for (const Fortran::semantics::Symbol *sym : allPrivatizedSymbols) { | ||
| if (sym->test(Fortran::semantics::Symbol::Flag::OmpFirstPrivate) && | ||
| sym->test(Fortran::semantics::Symbol::Flag::OmpLastPrivate)) | ||
| return true; | ||
|
|
@@ -289,12 +290,34 @@ void DataSharingProcessor::collectSymbolsInNestedRegions( | |
| eval.getNestedEvaluations()) { | ||
| if (nestedEval.hasNestedEvaluations()) { | ||
| if (nestedEval.isConstruct()) | ||
ergawy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| collectSymbolsInNestedRegions(nestedEval, flag, symbolsInNestedRegions); | ||
| else { | ||
| bool isOrderedConstruct = [&]() { | ||
|
There was a problem hiding this comment. @kiranchandramohan @luporl just to clarify this change: The changes here make sure that for the following input, !$omp do ordered
do i = 2, 10
!$omp ordered
a(i) = a(i-1) + 1
!$omp end ordered
end do
!$omp end doThere was a problem hiding this comment. It is not clear to me why the ordered construct is a special case. Will the same issue be resolved by a fix similar to #90671 ? There was a problem hiding this comment. I can describe the problem in more detail but cannot claim my solution is the proper one. For the above example program, this is the output of So, However, when you call This in turn, causes Since, That said, I am not sure this is the proper location for that kind of fix. But a similar fix to what was introduced in #90671 does not seem to work. Tried adding that in There was a problem hiding this comment. I think we can fix this separately. Could you check whether the same issue happens with the Critical construct? There was a problem hiding this comment. Thanks both for the review.
That indeed causes the same issue. I will add a similar "fix" and a test before merging. There was a problem hiding this comment. I have a small question. I am not able to map how control-flow is coming to I get semantic error as: There was a problem hiding this comment.
I think this error happens earlier in semantics because There was a problem hiding this comment. Thanks! Indeed it is from semantics. I was mainly checking if default privatization logic is being invoked uncharacteristically when it should not (i.e. when I'm looking into this |
||
| if (auto *ompConstruct = | ||
| nestedEval.getIf<parser::OpenMPConstruct>()) { | ||
| if (auto *ompBlockConstruct = | ||
| std::get_if<parser::OpenMPBlockConstruct>( | ||
| &ompConstruct->u)) { | ||
| const auto &beginBlockDirective = | ||
| std::get<Fortran::parser::OmpBeginBlockDirective>( | ||
| ompBlockConstruct->t); | ||
| const auto origDirective = | ||
| std::get<Fortran::parser::OmpBlockDirective>( | ||
| beginBlockDirective.t) | ||
| .v; | ||
|
|
||
| return origDirective == llvm::omp::Directive::OMPD_ordered; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| }(); | ||
|
|
||
| if (!isOrderedConstruct) | ||
| converter.collectSymbolSet(nestedEval, symbolsInNestedRegions, flag, | ||
| /*collectSymbols=*/true, | ||
| /*collectHostAssociatedSymbols=*/false); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -330,25 +353,46 @@ void DataSharingProcessor::collectSymbols( | |
| converter.collectSymbolSet(eval, allSymbols, flag, | ||
| /*collectSymbols=*/true, | ||
| /*collectHostAssociatedSymbols=*/true); | ||
|
|
||
| llvm::SetVector<const Fortran::semantics::Symbol *> symbolsInNestedRegions; | ||
| collectSymbolsInNestedRegions(eval, flag, symbolsInNestedRegions); | ||
| // Filter-out symbols that must not be privatized. | ||
| bool collectImplicit = flag == Fortran::semantics::Symbol::Flag::OmpImplicit; | ||
| bool collectPreDetermined = | ||
| flag == Fortran::semantics::Symbol::Flag::OmpPreDetermined; | ||
|
|
||
| auto isPrivatizable = [](const Fortran::semantics::Symbol &sym) -> bool { | ||
| return !Fortran::semantics::IsProcedure(sym) && | ||
| !sym.GetUltimate().has<Fortran::semantics::DerivedTypeDetails>() && | ||
| !sym.GetUltimate().has<Fortran::semantics::NamelistDetails>() && | ||
| !Fortran::semantics::IsImpliedDoIndex(sym.GetUltimate()); | ||
| }; | ||
|
|
||
| auto shouldCollectSymbol = [&](const Fortran::semantics::Symbol *sym) { | ||
| if (collectImplicit && | ||
| sym->test(Fortran::semantics::Symbol::Flag::OmpImplicit) && | ||
| !sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined)) | ||
| return true; | ||
|
|
||
| if (collectPreDetermined && | ||
| sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined)) | ||
| return true; | ||
|
|
||
| if (!sym->test(Fortran::semantics::Symbol::Flag::OmpImplicit) && | ||
| !sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined)) | ||
| return true; | ||
|
|
||
| return false; | ||
ergawy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| }; | ||
|
|
||
| for (const auto *sym : allSymbols) { | ||
| assert(curScope && "couldn't find current scope"); | ||
| if (isPrivatizable(*sym) && !symbolsInNestedRegions.contains(sym) && | ||
ergawy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| !explicitlyPrivatizedSymbols.contains(sym) && | ||
| shouldCollectSymbol(sym) && clauseScopes.contains(&sym->owner())) { | ||
| allPrivatizedSymbols.insert(sym); | ||
| symbols.insert(sym); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -365,6 +409,9 @@ void DataSharingProcessor::collectDefaultSymbols() { | |
| defaultSymbols); | ||
| } | ||
| } | ||
|
|
||
| for (auto *sym : defaultSymbols) | ||
| allPrivatizedSymbols.insert(sym); | ||
ergawy marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| void DataSharingProcessor::collectImplicitSymbols() { | ||
|
|
@@ -374,10 +421,15 @@ void DataSharingProcessor::collectImplicitSymbols() { | |
| implicitSymbols); | ||
| } | ||
|
|
||
| void DataSharingProcessor::collectPreDeterminedSymbols() { | ||
| collectSymbols(Fortran::semantics::Symbol::Flag::OmpPreDetermined, | ||
| preDeterminedSymbols); | ||
| } | ||
|
|
||
| void DataSharingProcessor::privatize( | ||
| mlir::omp::PrivateClauseOps *clauseOps, | ||
| llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *privateSyms) { | ||
| for (const Fortran::semantics::Symbol *sym : allPrivatizedSymbols) { | ||
| if (const auto *commonDet = | ||
| sym->detailsIf<Fortran::semantics::CommonBlockDetails>()) { | ||
| for (const auto &mem : commonDet->objects()) | ||
|
|
@@ -389,7 +441,7 @@ void DataSharingProcessor::privatize( | |
|
|
||
| void DataSharingProcessor::copyLastPrivatize(mlir::Operation *op) { | ||
| insertLastPrivateCompare(op); | ||
| for (const Fortran::semantics::Symbol *sym : allPrivatizedSymbols) | ||
| if (const auto *commonDet = | ||
| sym->detailsIf<Fortran::semantics::CommonBlockDetails>()) { | ||
| for (const auto &mem : commonDet->objects()) { | ||
|
|
@@ -400,20 +452,6 @@ void DataSharingProcessor::copyLastPrivatize(mlir::Operation *op) { | |
| } | ||
| } | ||
|
|
||
| void DataSharingProcessor::doPrivatize( | ||
| const Fortran::semantics::Symbol *sym, | ||
| mlir::omp::PrivateClauseOps *clauseOps, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.