[Flang][OpenMP] Per-sym checks to introduce barriers

[skatrak]

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.

[llvmbot]

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-fir-hlfir

Author: Sergio Afonso (skatrak)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/127074.diff

2 Files Affected:

• (modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+3-3)
• (modified) flang/lib/Lower/OpenMP/DataSharingProcessor.h (+1-1)

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;
 

[kiranchandramohan]

Can you add a test as the first commit and show the difference (removal of barrier) in the second commit along with the code in this patch?

[luporl]

Except for @kiranchandramohan's comments, LGTM.

[skatrak]

Can you add a test as the first commit and show the difference (removal of barrier) in the second commit along with the code in this patch?

Done, let me know if that works for you.

[kiranchandramohan]

LG. Thanks @skatrak

[tblah]

(late) +1

Thanks for fixing this. LGTM