[flang-commits] [flang] 8a3d7ce - [Flang][OpenMP] Per-sym checks to introduce barriers (#127074)

[via flang-commits]

Author: Sergio Afonso
Date: 2025-02-14T12:24:16Z
New Revision: 8a3d7cedb72add026c3f5bf7a82f0bf729b3f434

URL: https://github.com/llvm/llvm-project/commit/8a3d7cedb72add026c3f5bf7a82f0bf729b3f434
DIFF: https://github.com/llvm/llvm-project/commit/8a3d7cedb72add026c3f5bf7a82f0bf729b3f434.diff

LOG: [Flang][OpenMP] Per-sym checks to introduce barriers (#127074)

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 the same symbol both sets the flag and has
`lastprivate`.

Added: 
    flang/test/Lower/OpenMP/different_vars_lastprivate_barrier.f90

Modified: 
    flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
    flang/lib/Lower/OpenMP/DataSharingProcessor.h

Removed: 
    


################################################################################
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/
diff erent_vars_lastprivate_barrier.f90 b/flang/test/Lower/OpenMP/
diff erent_vars_lastprivate_barrier.f90
new file mode 100644
index 0000000000000..0f04977754291
--- /dev/null
+++ b/flang/test/Lower/OpenMP/
diff erent_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-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.terminator
+! CHECK:        }