[flang-commits] [flang] 9cf8752 - [flang][OpenMP] Handle symbols on composite simd with multiple privatizers (#155640)
via flang-commits
flang-commits at lists.llvm.org
Thu Aug 28 07:58:40 PDT 2025
Author: Kajetan Puchalski
Date: 2025-08-28T15:58:29+01:00
New Revision: 9cf8752ccfd194c3fa1cda641db2e3c77aa4915c
URL: https://github.com/llvm/llvm-project/commit/9cf8752ccfd194c3fa1cda641db2e3c77aa4915c
DIFF: https://github.com/llvm/llvm-project/commit/9cf8752ccfd194c3fa1cda641db2e3c77aa4915c.diff
LOG: [flang][OpenMP] Handle symbols on composite simd with multiple privatizers (#155640)
In some cases, a clause on a composite simd construct applied to simd
can be using a symbol that is also used by another privatizer, not
applied to simd. Correctly handle this scenario by checking which
directive the privatizer is being generated for while determining
whether to emit the copy region.
Fixes #155195.
Signed-off-by: Kajetan Puchalski <kajetan.puchalski at arm.com>
Added:
Modified:
flang/include/flang/Lower/Support/Utils.h
flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
flang/lib/Lower/OpenMP/DataSharingProcessor.h
flang/lib/Lower/OpenMP/OpenMP.cpp
flang/lib/Lower/Support/Utils.cpp
flang/test/Lower/OpenMP/wsloop-simd.f90
Removed:
################################################################################
diff --git a/flang/include/flang/Lower/Support/Utils.h b/flang/include/flang/Lower/Support/Utils.h
index 697bb05ec8cee..eac5cad91f608 100644
--- a/flang/include/flang/Lower/Support/Utils.h
+++ b/flang/include/flang/Lower/Support/Utils.h
@@ -102,7 +102,8 @@ void privatizeSymbol(
lower::SymMap &symTable,
llvm::SetVector<const semantics::Symbol *> &allPrivatizedSymbols,
llvm::SmallPtrSet<const semantics::Symbol *, 16> &mightHaveReadHostSym,
- const semantics::Symbol *symToPrivatize, OperandsStructType *clauseOps);
+ const semantics::Symbol *symToPrivatize, OperandsStructType *clauseOps,
+ std::optional<llvm::omp::Directive> dir = std::nullopt);
} // end namespace Fortran::lower
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index c4054ef0fc961..146a252b049ec 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -90,13 +90,14 @@ DataSharingProcessor::DataSharingProcessor(lower::AbstractConverter &converter,
isTargetPrivatization) {}
void DataSharingProcessor::processStep1(
- mlir::omp::PrivateClauseOps *clauseOps) {
+ mlir::omp::PrivateClauseOps *clauseOps,
+ std::optional<llvm::omp::Directive> dir) {
collectSymbolsForPrivatization();
collectDefaultSymbols();
collectImplicitSymbols();
collectPreDeterminedSymbols();
- privatize(clauseOps);
+ privatize(clauseOps, dir);
insertBarrier(clauseOps);
}
@@ -581,14 +582,15 @@ void DataSharingProcessor::collectPreDeterminedSymbols() {
preDeterminedSymbols);
}
-void DataSharingProcessor::privatize(mlir::omp::PrivateClauseOps *clauseOps) {
+void DataSharingProcessor::privatize(mlir::omp::PrivateClauseOps *clauseOps,
+ std::optional<llvm::omp::Directive> dir) {
for (const semantics::Symbol *sym : allPrivatizedSymbols) {
if (const auto *commonDet =
sym->detailsIf<semantics::CommonBlockDetails>()) {
for (const auto &mem : commonDet->objects())
- privatizeSymbol(&*mem, clauseOps);
+ privatizeSymbol(&*mem, clauseOps, dir);
} else
- privatizeSymbol(sym, clauseOps);
+ privatizeSymbol(sym, clauseOps, dir);
}
}
@@ -607,7 +609,8 @@ void DataSharingProcessor::copyLastPrivatize(mlir::Operation *op) {
void DataSharingProcessor::privatizeSymbol(
const semantics::Symbol *symToPrivatize,
- mlir::omp::PrivateClauseOps *clauseOps) {
+ mlir::omp::PrivateClauseOps *clauseOps,
+ std::optional<llvm::omp::Directive> dir) {
if (!useDelayedPrivatization) {
cloneSymbol(symToPrivatize);
copyFirstPrivateSymbol(symToPrivatize);
@@ -617,7 +620,7 @@ void DataSharingProcessor::privatizeSymbol(
Fortran::lower::privatizeSymbol<mlir::omp::PrivateClauseOp,
mlir::omp::PrivateClauseOps>(
converter, firOpBuilder, symTable, allPrivatizedSymbols,
- mightHaveReadHostSym, symToPrivatize, clauseOps);
+ mightHaveReadHostSym, symToPrivatize, clauseOps, dir);
}
} // namespace omp
} // namespace lower
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.h b/flang/lib/Lower/OpenMP/DataSharingProcessor.h
index 00b2d95bab224..f6aa8652e3534 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.h
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.h
@@ -126,7 +126,8 @@ class DataSharingProcessor {
void collectDefaultSymbols();
void collectImplicitSymbols();
void collectPreDeterminedSymbols();
- void privatize(mlir::omp::PrivateClauseOps *clauseOps);
+ void privatize(mlir::omp::PrivateClauseOps *clauseOps,
+ std::optional<llvm::omp::Directive> dir = std::nullopt);
void copyLastPrivatize(mlir::Operation *op);
void insertLastPrivateCompare(mlir::Operation *op);
void cloneSymbol(const semantics::Symbol *sym);
@@ -167,7 +168,8 @@ class DataSharingProcessor {
// Step2 performs the copying for lastprivates and requires knowledge of the
// MLIR operation to insert the last private update. Step2 adds
// dealocation code as well.
- void processStep1(mlir::omp::PrivateClauseOps *clauseOps = nullptr);
+ void processStep1(mlir::omp::PrivateClauseOps *clauseOps = nullptr,
+ std::optional<llvm::omp::Directive> dir = std::nullopt);
void processStep2(mlir::Operation *op, bool isLoop);
void pushLoopIV(mlir::Value iv) { loopIVs.push_back(iv); }
@@ -184,7 +186,8 @@ class DataSharingProcessor {
}
void privatizeSymbol(const semantics::Symbol *symToPrivatize,
- mlir::omp::PrivateClauseOps *clauseOps);
+ mlir::omp::PrivateClauseOps *clauseOps,
+ std::optional<llvm::omp::Directive> dir = std::nullopt);
};
} // namespace omp
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 9df2f1d79aa67..6427133cea71e 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3257,7 +3257,7 @@ static mlir::omp::WsloopOp genCompositeDoSimd(
DataSharingProcessor simdItemDSP(converter, semaCtx, simdItem->clauses, eval,
/*shouldCollectPreDeterminedSymbols=*/true,
/*useDelayedPrivatization=*/true, symTable);
- simdItemDSP.processStep1(&simdClauseOps);
+ simdItemDSP.processStep1(&simdClauseOps, simdItem->id);
// Pass the innermost leaf construct's clauses because that's where COLLAPSE
// is placed by construct decomposition.
diff --git a/flang/lib/Lower/Support/Utils.cpp b/flang/lib/Lower/Support/Utils.cpp
index 0cdb03beb72a2..1b4d37e9798a9 100644
--- a/flang/lib/Lower/Support/Utils.cpp
+++ b/flang/lib/Lower/Support/Utils.cpp
@@ -655,7 +655,8 @@ void privatizeSymbol(
lower::SymMap &symTable,
llvm::SetVector<const semantics::Symbol *> &allPrivatizedSymbols,
llvm::SmallPtrSet<const semantics::Symbol *, 16> &mightHaveReadHostSym,
- const semantics::Symbol *symToPrivatize, OperandsStructType *clauseOps) {
+ const semantics::Symbol *symToPrivatize, OperandsStructType *clauseOps,
+ std::optional<llvm::omp::Directive> dir) {
constexpr bool isDoConcurrent =
std::is_same_v<OpType, fir::LocalitySpecifierOp>;
mlir::OpBuilder::InsertPoint dcIP;
@@ -676,6 +677,13 @@ void privatizeSymbol(
bool emitCopyRegion =
symToPrivatize->test(semantics::Symbol::Flag::OmpFirstPrivate) ||
symToPrivatize->test(semantics::Symbol::Flag::LocalityLocalInit);
+ // A symbol attached to the simd directive can have the firstprivate flag set
+ // on it when it is also used in a non-firstprivate privatization clause.
+ // For instance: $omp do simd lastprivate(a) firstprivate(a)
+ // We cannot apply the firstprivate privatizer to simd, so make sure we do
+ // not emit the copy region when dealing with the SIMD directive.
+ if (dir && dir == llvm::omp::Directive::OMPD_simd)
+ emitCopyRegion = false;
mlir::Value privVal = hsb.getAddr();
mlir::Type allocType = privVal.getType();
@@ -848,7 +856,8 @@ privatizeSymbol<mlir::omp::PrivateClauseOp, mlir::omp::PrivateClauseOps>(
llvm::SetVector<const semantics::Symbol *> &allPrivatizedSymbols,
llvm::SmallPtrSet<const semantics::Symbol *, 16> &mightHaveReadHostSym,
const semantics::Symbol *symToPrivatize,
- mlir::omp::PrivateClauseOps *clauseOps);
+ mlir::omp::PrivateClauseOps *clauseOps,
+ std::optional<llvm::omp::Directive> dir);
template void
privatizeSymbol<fir::LocalitySpecifierOp, fir::LocalitySpecifierOperands>(
@@ -857,6 +866,7 @@ privatizeSymbol<fir::LocalitySpecifierOp, fir::LocalitySpecifierOperands>(
llvm::SetVector<const semantics::Symbol *> &allPrivatizedSymbols,
llvm::SmallPtrSet<const semantics::Symbol *, 16> &mightHaveReadHostSym,
const semantics::Symbol *symToPrivatize,
- fir::LocalitySpecifierOperands *clauseOps);
+ fir::LocalitySpecifierOperands *clauseOps,
+ std::optional<llvm::omp::Directive> dir);
} // end namespace Fortran::lower
diff --git a/flang/test/Lower/OpenMP/wsloop-simd.f90 b/flang/test/Lower/OpenMP/wsloop-simd.f90
index d26e93d9a5113..03e35de04cace 100644
--- a/flang/test/Lower/OpenMP/wsloop-simd.f90
+++ b/flang/test/Lower/OpenMP/wsloop-simd.f90
@@ -85,3 +85,20 @@ subroutine do_simd_private()
tmp = tmp + 1
end do
end subroutine do_simd_private
+
+! CHECK-LABEL: func.func @_QPdo_simd_lastprivate_firstprivate(
+subroutine do_simd_lastprivate_firstprivate()
+ integer :: a
+ ! CHECK: omp.wsloop
+ ! CHECK-SAME: private(@[[FIRSTPRIVATE_A_SYM:.*]] %{{.*}} -> %[[FIRSTPRIVATE_A:.*]] : !fir.ref<i32>)
+ ! CHECK-NEXT: omp.simd
+ ! CHECK-SAME: private(@[[PRIVATE_A_SYM:.*]] %{{.*}} -> %[[PRIVATE_A:.*]], @[[PRIVATE_I_SYM:.*]] %{{.*}} -> %[[PRIVATE_I:.*]] : !fir.ref<i32>, !fir.ref<i32>)
+ !$omp do simd lastprivate(a) firstprivate(a)
+ do i = 1, 10
+ ! CHECK: %[[FIRSTPRIVATE_A_DECL:.*]]:2 = hlfir.declare %[[FIRSTPRIVATE_A]]
+ ! CHECK: %[[PRIVATE_A_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_A]]
+ ! CHECK: %[[PRIVATE_I_DECL:.*]]:2 = hlfir.declare %[[PRIVATE_I]]
+ a = a + 1
+ end do
+ !$omp end do simd
+end subroutine do_simd_lastprivate_firstprivate
More information about the flang-commits
mailing list