[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 semantics::Symbol *sym : allPrivatizedSymbols) | ||
| if (semantics::IsAllocatable(sym->GetUltimate())) { | ||
| if (!useDelayedPrivatization) { | ||
| converter.createHostAssociateVarCloneDealloc(*sym); | ||
| | @@ -92,10 +93,6 @@ void DataSharingProcessor::insertDeallocs() { | |
| } | ||
| | ||
| void DataSharingProcessor::cloneSymbol(const semantics::Symbol *sym) { | ||
| bool success = converter.createHostAssociateVarClone(*sym); | ||
| (void)success; | ||
| assert(success && "Privatization failed due to existing binding"); | ||
| | @@ -126,20 +123,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"); | ||
| } | ||
| | @@ -149,7 +150,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 semantics::Symbol *sym : allPrivatizedSymbols) { | ||
| if (sym->test(semantics::Symbol::Flag::OmpFirstPrivate) && | ||
| sym->test(semantics::Symbol::Flag::OmpLastPrivate)) | ||
| return true; | ||
| | @@ -283,10 +284,40 @@ void DataSharingProcessor::collectSymbolsInNestedRegions( | |
| if (nestedEval.isConstruct()) | ||
| // Recursively look for OpenMP constructs within `nestedEval`'s region | ||
| collectSymbolsInNestedRegions(nestedEval, flag, symbolsInNestedRegions); | ||
| else { | ||
| bool isOrderedConstruct = [&]() { | ||
| Member Author 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 do Contributor There 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 ? Member Author 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 Contributor 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? Contributor Member Author 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. Contributor 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: Member There was a problem hiding this comment.
I think this error happens earlier in semantics because Contributor 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<parser::OmpBeginBlockDirective>( | ||
| ompBlockConstruct->t); | ||
| const auto origDirective = | ||
| std::get<parser::OmpBlockDirective>(beginBlockDirective.t).v; | ||
| | ||
| return origDirective == llvm::omp::Directive::OMPD_ordered; | ||
| } | ||
| } | ||
| | ||
| return false; | ||
| }(); | ||
| | ||
| bool isCriticalConstruct = [&]() { | ||
| if (auto *ompConstruct = | ||
| nestedEval.getIf<parser::OpenMPConstruct>()) { | ||
| return std::get_if<parser::OpenMPCriticalConstruct>( | ||
| &ompConstruct->u) != nullptr; | ||
| } | ||
| return false; | ||
| }(); | ||
| | ||
| if (!isOrderedConstruct && !isCriticalConstruct) | ||
| converter.collectSymbolSet(nestedEval, symbolsInNestedRegions, flag, | ||
| /*collectSymbols=*/true, | ||
| /*collectHostAssociatedSymbols=*/false); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| | @@ -322,24 +353,39 @@ void DataSharingProcessor::collectSymbols( | |
| converter.collectSymbolSet(eval, allSymbols, flag, | ||
| /*collectSymbols=*/true, | ||
| /*collectHostAssociatedSymbols=*/true); | ||
| | ||
| llvm::SetVector<const semantics::Symbol *> symbolsInNestedRegions; | ||
| collectSymbolsInNestedRegions(eval, flag, symbolsInNestedRegions); | ||
| // Filter-out symbols that must not be privatized. | ||
| bool collectImplicit = flag == semantics::Symbol::Flag::OmpImplicit; | ||
| bool collectPreDetermined = flag == semantics::Symbol::Flag::OmpPreDetermined; | ||
| | ||
| auto isPrivatizable = [](const semantics::Symbol &sym) -> bool { | ||
| return !semantics::IsProcedure(sym) && | ||
| !sym.GetUltimate().has<semantics::DerivedTypeDetails>() && | ||
| !sym.GetUltimate().has<semantics::NamelistDetails>() && | ||
| !semantics::IsImpliedDoIndex(sym.GetUltimate()); | ||
| }; | ||
| | ||
| auto shouldCollectSymbol = [&](const semantics::Symbol *sym) { | ||
| if (collectImplicit) | ||
| return sym->test(semantics::Symbol::Flag::OmpImplicit); | ||
| | ||
| if (collectPreDetermined) | ||
| return sym->test(semantics::Symbol::Flag::OmpPreDetermined); | ||
| | ||
| return !sym->test(semantics::Symbol::Flag::OmpImplicit) && | ||
| !sym->test(semantics::Symbol::Flag::OmpPreDetermined); | ||
| }; | ||
| | ||
| for (const auto *sym : allSymbols) { | ||
| assert(curScope && "couldn't find current scope"); | ||
| if (isPrivatizable(*sym) && !symbolsInNestedRegions.contains(sym) && | ||
| !explicitlyPrivatizedSymbols.contains(sym) && | ||
| shouldCollectSymbol(sym) && clauseScopes.contains(&sym->owner())) { | ||
| allPrivatizedSymbols.insert(sym); | ||
| symbols.insert(sym); | ||
| } | ||
| } | ||
| } | ||
| | ||
| | @@ -363,10 +409,16 @@ void DataSharingProcessor::collectImplicitSymbols() { | |
| collectSymbols(semantics::Symbol::Flag::OmpImplicit, implicitSymbols); | ||
| } | ||
| | ||
| void DataSharingProcessor::collectPreDeterminedSymbols() { | ||
| if (shouldCollectPreDeterminedSymbols) | ||
| collectSymbols(semantics::Symbol::Flag::OmpPreDetermined, | ||
| preDeterminedSymbols); | ||
| } | ||
| | ||
| void DataSharingProcessor::privatize( | ||
| mlir::omp::PrivateClauseOps *clauseOps, | ||
| llvm::SmallVectorImpl<const semantics::Symbol *> *privateSyms) { | ||
| for (const semantics::Symbol *sym : allPrivatizedSymbols) { | ||
| if (const auto *commonDet = | ||
| sym->detailsIf<semantics::CommonBlockDetails>()) { | ||
| for (const auto &mem : commonDet->objects()) | ||
| | @@ -378,7 +430,7 @@ void DataSharingProcessor::privatize( | |
| | ||
| void DataSharingProcessor::copyLastPrivatize(mlir::Operation *op) { | ||
| insertLastPrivateCompare(op); | ||
| for (const semantics::Symbol *sym : allPrivatizedSymbols) | ||
| if (const auto *commonDet = | ||
| sym->detailsIf<semantics::CommonBlockDetails>()) { | ||
| for (const auto &mem : commonDet->objects()) { | ||
| | @@ -389,20 +441,6 @@ void DataSharingProcessor::copyLastPrivatize(mlir::Operation *op) { | |
| } | ||
| } | ||
| | ||
| void DataSharingProcessor::doPrivatize( | ||
| const semantics::Symbol *sym, mlir::omp::PrivateClauseOps *clauseOps, | ||
| llvm::SmallVectorImpl<const semantics::Symbol *> *privateSyms) { | ||
| | ||
Uh oh!
There was an error while loading. Please reload this page.