From 59b534dd9c56022d92d7d9ecbf71878bc578d800 Mon Sep 17 00:00:00 2001 From: Sergio Afonso Date: Mon, 26 Aug 2024 12:12:24 +0100 Subject: [PATCH] [Flang][OpenMP] Simplify privatization This patch removes some redundant information from the `DataSharingProcessor` class that was initially introduced to support wrapper-based representation of loop constructs and that is no longer needed. Usage of this class is also simplified as a result. --- .../lib/Lower/OpenMP/DataSharingProcessor.cpp | 27 ++++++++++--------- flang/lib/Lower/OpenMP/DataSharingProcessor.h | 19 +++++-------- flang/lib/Lower/OpenMP/OpenMP.cpp | 26 ++++-------------- 3 files changed, 26 insertions(+), 46 deletions(-) diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp index c49682d450f318..d692d354ff8aff 100644 --- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp +++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp @@ -56,17 +56,18 @@ void DataSharingProcessor::processStep1() { collectPreDeterminedSymbols(); } -void DataSharingProcessor::processStep2() { +void DataSharingProcessor::processStep2( + mlir::omp::PrivateClauseOps *clauseOps) { if (privatizationDone) return; - privatize(); + privatize(clauseOps); insertBarrier(); privatizationDone = true; } void DataSharingProcessor::processStep3(mlir::Operation *op, bool isLoop) { - // 'sections' lastprivate is handled by genOMP() + // 'sections' lastprivate is handled by genOMP() if (!mlir::isa(op)) { insPt = firOpBuilder.saveInsertionPoint(); copyLastPrivatize(op); @@ -74,7 +75,7 @@ void DataSharingProcessor::processStep3(mlir::Operation *op, bool isLoop) { } if (isLoop) { - // push deallocs out of the loop + // push deallocs out of the loop firOpBuilder.setInsertionPointAfter(op); insertDeallocs(); } else { @@ -479,14 +480,14 @@ void DataSharingProcessor::collectPreDeterminedSymbols() { preDeterminedSymbols); } -void DataSharingProcessor::privatize() { +void DataSharingProcessor::privatize(mlir::omp::PrivateClauseOps *clauseOps) { for (const semantics::Symbol *sym : allPrivatizedSymbols) { if (const auto *commonDet = sym->detailsIf()) { for (const auto &mem : commonDet->objects()) - doPrivatize(&*mem); + doPrivatize(&*mem, clauseOps); } else { - doPrivatize(sym); + doPrivatize(sym, clauseOps); } } } @@ -504,7 +505,8 @@ void DataSharingProcessor::copyLastPrivatize(mlir::Operation *op) { } } -void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym) { +void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym, + mlir::omp::PrivateClauseOps *clauseOps) { if (!useDelayedPrivatization) { cloneSymbol(sym); copyFirstPrivateSymbol(sym); @@ -615,11 +617,10 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym) { return result; }(); - privateClauseOps.privateSyms.push_back( - mlir::SymbolRefAttr::get(privatizerOp)); - privateClauseOps.privateVars.push_back(hsb.getAddr()); - delayedPrivSyms.push_back(sym); - + if (clauseOps) { + clauseOps->privateSyms.push_back(mlir::SymbolRefAttr::get(privatizerOp)); + clauseOps->privateVars.push_back(hsb.getAddr()); + } symToPrivatizer[sym] = privatizerOp; } diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.h b/flang/lib/Lower/OpenMP/DataSharingProcessor.h index 1f1aa3c5eb7252..67b3599793d96c 100644 --- a/flang/lib/Lower/OpenMP/DataSharingProcessor.h +++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.h @@ -91,8 +91,6 @@ class DataSharingProcessor { lower::SymMap *symTable; OMPConstructSymbolVisitor visitor; - mlir::omp::PrivateClauseOps privateClauseOps; - llvm::SmallVector delayedPrivSyms; bool privatizationDone = false; bool needBarrier(); @@ -109,10 +107,9 @@ class DataSharingProcessor { void collectDefaultSymbols(); void collectImplicitSymbols(); void collectPreDeterminedSymbols(); - void privatize(); - void defaultPrivatize(); - void implicitPrivatize(); - void doPrivatize(const semantics::Symbol *sym); + void privatize(mlir::omp::PrivateClauseOps *clauseOps); + void doPrivatize(const semantics::Symbol *sym, + mlir::omp::PrivateClauseOps *clauseOps); void copyLastPrivatize(mlir::Operation *op); void insertLastPrivateCompare(mlir::Operation *op); void cloneSymbol(const semantics::Symbol *sym); @@ -157,7 +154,7 @@ class DataSharingProcessor { // before the operation is created since the bounds of the MLIR OpenMP // operation can be privatised. void processStep1(); - void processStep2(); + void processStep2(mlir::omp::PrivateClauseOps *clauseOps = nullptr); void processStep3(mlir::Operation *op, bool isLoop); void pushLoopIV(mlir::Value iv) { loopIVs.push_back(iv); } @@ -167,12 +164,10 @@ class DataSharingProcessor { return allPrivatizedSymbols; } - const mlir::omp::PrivateClauseOps &getPrivateClauseOps() const { - return privateClauseOps; - } - llvm::ArrayRef getDelayedPrivSyms() const { - return delayedPrivSyms; + return useDelayedPrivatization + ? allPrivatizedSymbols.getArrayRef() + : llvm::ArrayRef(); } }; diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index 08bbb99ead3237..5a4799a2f88ed1 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -1946,11 +1946,7 @@ genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable, lower::omp::isLastItemInQueue(item, queue), /*useDelayedPrivatization=*/true, &symTable); dsp.processStep1(); - dsp.processStep2(); - - const auto &privateClauseOps = dsp.getPrivateClauseOps(); - clauseOps.privateVars = privateClauseOps.privateVars; - clauseOps.privateSyms = privateClauseOps.privateSyms; + dsp.processStep2(&clauseOps); auto genRegionEntryCB = [&](mlir::Operation *op) { auto parallelOp = llvm::cast(op); @@ -2026,12 +2022,6 @@ static mlir::omp::ParallelOp genParallelCompositeOp( DataSharingProcessor &dsp) { fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); - if (enableDelayedPrivatization) { - const auto &privateClauseOps = dsp.getPrivateClauseOps(); - clauseOps.privateVars = privateClauseOps.privateVars; - clauseOps.privateSyms = privateClauseOps.privateSyms; - } - // Create omp.parallel operation. auto parallelOp = firOpBuilder.create(loc, clauseOps); parallelOp.setComposite(/*val=*/true); @@ -2268,14 +2258,8 @@ genTargetOp(lower::AbstractConverter &converter, lower::SymMap &symTable, lower::omp::isLastItemInQueue(item, queue), enableDelayedPrivatizationStaging, &symTable); dsp.processStep1(); - - if (enableDelayedPrivatizationStaging) { - dsp.processStep2(); - - const auto &privateClauseOps = dsp.getPrivateClauseOps(); - clauseOps.privateVars = privateClauseOps.privateVars; - clauseOps.privateSyms = privateClauseOps.privateSyms; - } + if (enableDelayedPrivatizationStaging) + dsp.processStep2(&clauseOps); // 5.8.1 Implicit Data-Mapping Attribute Rules // The following code follows the implicit data-mapping rules to map all the @@ -2711,7 +2695,7 @@ static void genCompositeDistributeParallelDo( /*shouldCollectPreDeterminedSymbols=*/true, /*useDelayedPrivatization=*/true, &symTable); dsp.processStep1(); - dsp.processStep2(); + dsp.processStep2(enableDelayedPrivatization ? ¶llelClauseOps : nullptr); genParallelCompositeOp(converter, semaCtx, parallelItem->clauses, eval, loc, parallelClauseOps, numThreadsClauseOps, @@ -2791,7 +2775,7 @@ static void genCompositeDistributeParallelDoSimd( /*shouldCollectPreDeterminedSymbols=*/true, /*useDelayedPrivatization=*/true, &symTable); dsp.processStep1(); - dsp.processStep2(); + dsp.processStep2(enableDelayedPrivatization ? ¶llelClauseOps : nullptr); genParallelCompositeOp(converter, semaCtx, parallelItem->clauses, eval, loc, parallelClauseOps, numThreadsClauseOps,