[flang-commits] [flang] [Flang][OpenMP] Per-sym checks to introduce barriers (PR #127074)
Sergio Afonso via flang-commits
flang-commits at lists.llvm.org
Fri Feb 14 03:07:37 PST 2025
https://github.com/skatrak updated https://github.com/llvm/llvm-project/pull/127074
>From 32001b76a4ff82d844ea59e51d4bf53b2b1946f9 Mon Sep 17 00:00:00 2001
From: Sergio Afonso <safonsof at amd.com>
Date: Fri, 14 Feb 2025 11:05:16 +0000
Subject: [PATCH 1/2] Add test showing redundant barrier
---
.../different_vars_lastprivate_barrier.f90 | 40 +++++++++++++++++++
1 file changed, 40 insertions(+)
create mode 100644 flang/test/Lower/OpenMP/different_vars_lastprivate_barrier.f90
diff --git a/flang/test/Lower/OpenMP/different_vars_lastprivate_barrier.f90 b/flang/test/Lower/OpenMP/different_vars_lastprivate_barrier.f90
new file mode 100644
index 0000000000000..c6bab36b737a0
--- /dev/null
+++ b/flang/test/Lower/OpenMP/different_vars_lastprivate_barrier.f90
@@ -0,0 +1,40 @@
+! RUN: %flang_fc1 -fopenmp -mmlir --openmp-enable-delayed-privatization-staging=true -emit-hlfir %s -o - | FileCheck %s
+
+subroutine first_and_lastprivate
+ integer i
+ integer, dimension(3) :: var
+
+ !$omp parallel do lastprivate(i) private(var)
+ do i=1,1
+ end do
+ !$omp end parallel do
+end subroutine
+
+! CHECK: omp.private {type = private} @[[VAR_PRIVATIZER:.*Evar_private_box_3xi32]] : [[BOX_TYPE:!fir\.box<!fir\.array<3xi32>>]] init {
+! CHECK-NEXT: ^bb0(%[[ORIG_REF:.*]]: {{.*}}, %[[PRIV_REF:.*]]: {{.*}}):
+! CHECK: %[[ORIG_VAL:.*]] = fir.load %[[ORIG_REF]]
+! CHECK: %[[BOX_DIMS:.*]]:3 = fir.box_dims %[[ORIG_VAL]], %{{.*}} : ([[BOX_TYPE]], index) -> (index, index, index)
+! CHECK: %[[SHAPE_SHIFT:.*]] = fir.shape_shift %[[BOX_DIMS]]#0, %[[BOX_DIMS]]#1
+! CHECK: %[[EMBOX:.*]] = fir.embox %{{.*}}(%[[SHAPE_SHIFT]]) : {{.*}} -> [[BOX_TYPE]]
+! CHECK: fir.store %[[EMBOX]] to %[[PRIV_REF]]
+! CHECK: omp.yield(%[[PRIV_REF]] : !fir.ref<[[BOX_TYPE]]>)
+! CHECK: }
+
+! CHECK: omp.private {type = private} @[[I_PRIVATIZER:.*Ei_private_i32]] : i32
+
+! CHECK: func.func @{{.*}}first_and_lastprivate()
+! CHECK: %[[ORIG_I_DECL:.*]]:2 = hlfir.declare {{.*}} {uniq_name = "{{.*}}Ei"}
+! CHECK: omp.parallel {
+! CHECK: omp.barrier
+! CHECK: omp.wsloop private(@[[I_PRIVATIZER]] %[[ORIG_I_DECL]]#0 -> %[[I_ARG:.*]], @[[VAR_PRIVATIZER]] {{.*}}) {
+! CHECK: omp.loop_nest {{.*}} {
+! CHECK: %[[PRIV_I_DECL:.*]]:2 = hlfir.declare %[[I_ARG]] {uniq_name = "{{.*}}Ei"}
+! CHECK: fir.if %{{.*}} {
+! CHECK: %[[PRIV_I_VAL:.*]] = fir.load %[[PRIV_I_DECL]]#0 : !fir.ref<i32>
+! CHECK: hlfir.assign %[[PRIV_I_VAL]] to %[[ORIG_I_DECL]]#0
+! CHECK: }
+! CHECK: omp.yield
+! CHECK: }
+! CHECK: }
+! CHECK: omp.terminator
+! CHECK: }
>From c11820a348eac1b3f094ac2b054cea874b953d47 Mon Sep 17 00:00:00 2001
From: Sergio Afonso <safonsof at amd.com>
Date: Thu, 13 Feb 2025 15:30:12 +0000
Subject: [PATCH 2/2] [Flang][OpenMP] Per-sym checks to introduce barriers
Whenever there is a `lastprivate` variable and another unrelated variable sets
the `mightHaveReadHostSym` flag during Flang lowering privatization, this will
result in the insertion of a barrier.
This patch modifies this behavior such that this barrier will not be inserted
unless it has to be the same variable to set the flag and be `lastprivate` for
this to be done.
---
.../lib/Lower/OpenMP/DataSharingProcessor.cpp | 6 ++--
flang/lib/Lower/OpenMP/DataSharingProcessor.h | 2 +-
.../different_vars_lastprivate_barrier.f90 | 28 +++++++++----------
3 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 55f543ca38178..d13f101f516e7 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -166,7 +166,7 @@ void DataSharingProcessor::cloneSymbol(const semantics::Symbol *sym) {
if (needInitClone()) {
Fortran::lower::initializeCloneAtRuntime(converter, *sym, symTable);
- mightHaveReadHostSym = true;
+ mightHaveReadHostSym.insert(sym);
}
}
@@ -222,7 +222,7 @@ bool DataSharingProcessor::needBarrier() {
for (const semantics::Symbol *sym : allPrivatizedSymbols) {
if (sym->test(semantics::Symbol::Flag::OmpLastPrivate) &&
(sym->test(semantics::Symbol::Flag::OmpFirstPrivate) ||
- mightHaveReadHostSym))
+ mightHaveReadHostSym.contains(sym)))
return true;
}
return false;
@@ -594,7 +594,7 @@ void DataSharingProcessor::doPrivatize(const semantics::Symbol *sym,
// TODO: currently there are false positives from dead uses of the mold
// arg
if (!result.getInitMoldArg().getUses().empty())
- mightHaveReadHostSym = true;
+ mightHaveReadHostSym.insert(sym);
}
// Populate the `copy` region if this is a `firstprivate`.
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.h b/flang/lib/Lower/OpenMP/DataSharingProcessor.h
index 8e15c6d260389..54a42fd199831 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.h
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.h
@@ -86,7 +86,7 @@ class DataSharingProcessor {
lower::pft::Evaluation &eval;
bool shouldCollectPreDeterminedSymbols;
bool useDelayedPrivatization;
- bool mightHaveReadHostSym = false;
+ llvm::SmallSet<const semantics::Symbol *, 16> mightHaveReadHostSym;
lower::SymMap &symTable;
OMPConstructSymbolVisitor visitor;
diff --git a/flang/test/Lower/OpenMP/different_vars_lastprivate_barrier.f90 b/flang/test/Lower/OpenMP/different_vars_lastprivate_barrier.f90
index c6bab36b737a0..0f04977754291 100644
--- a/flang/test/Lower/OpenMP/different_vars_lastprivate_barrier.f90
+++ b/flang/test/Lower/OpenMP/different_vars_lastprivate_barrier.f90
@@ -22,19 +22,19 @@ subroutine first_and_lastprivate
! CHECK: omp.private {type = private} @[[I_PRIVATIZER:.*Ei_private_i32]] : i32
-! CHECK: func.func @{{.*}}first_and_lastprivate()
-! CHECK: %[[ORIG_I_DECL:.*]]:2 = hlfir.declare {{.*}} {uniq_name = "{{.*}}Ei"}
-! CHECK: omp.parallel {
-! CHECK: omp.barrier
-! CHECK: omp.wsloop private(@[[I_PRIVATIZER]] %[[ORIG_I_DECL]]#0 -> %[[I_ARG:.*]], @[[VAR_PRIVATIZER]] {{.*}}) {
-! CHECK: omp.loop_nest {{.*}} {
-! CHECK: %[[PRIV_I_DECL:.*]]:2 = hlfir.declare %[[I_ARG]] {uniq_name = "{{.*}}Ei"}
-! CHECK: fir.if %{{.*}} {
-! CHECK: %[[PRIV_I_VAL:.*]] = fir.load %[[PRIV_I_DECL]]#0 : !fir.ref<i32>
-! CHECK: hlfir.assign %[[PRIV_I_VAL]] to %[[ORIG_I_DECL]]#0
+! CHECK: func.func @{{.*}}first_and_lastprivate()
+! CHECK: %[[ORIG_I_DECL:.*]]:2 = hlfir.declare {{.*}} {uniq_name = "{{.*}}Ei"}
+! CHECK: omp.parallel {
+! CHECK-NOT: omp.barrier
+! CHECK: omp.wsloop private(@[[I_PRIVATIZER]] %[[ORIG_I_DECL]]#0 -> %[[I_ARG:.*]], @[[VAR_PRIVATIZER]] {{.*}}) {
+! CHECK: omp.loop_nest {{.*}} {
+! CHECK: %[[PRIV_I_DECL:.*]]:2 = hlfir.declare %[[I_ARG]] {uniq_name = "{{.*}}Ei"}
+! CHECK: fir.if %{{.*}} {
+! CHECK: %[[PRIV_I_VAL:.*]] = fir.load %[[PRIV_I_DECL]]#0 : !fir.ref<i32>
+! CHECK: hlfir.assign %[[PRIV_I_VAL]] to %[[ORIG_I_DECL]]#0
+! CHECK: }
+! CHECK: omp.yield
+! CHECK: }
! CHECK: }
-! CHECK: omp.yield
+! CHECK: omp.terminator
! CHECK: }
-! CHECK: }
-! CHECK: omp.terminator
-! CHECK: }
More information about the flang-commits
mailing list