[flang-commits] [flang] 8650c80 - [flang][OpenMP] Do not skip privatization of linear variable if it is OmpPreDetermined (#144315)

[via flang-commits]

Author: NimishMishra
Date: 2025-06-20T04:52:40-07:00
New Revision: 8650c801381d5640018a2fab32932ee41ad27797

URL: https://github.com/llvm/llvm-project/commit/8650c801381d5640018a2fab32932ee41ad27797
DIFF: https://github.com/llvm/llvm-project/commit/8650c801381d5640018a2fab32932ee41ad27797.diff

LOG: [flang][OpenMP] Do not skip privatization of linear variable if it is OmpPreDetermined (#144315)

Current implementation of linear clause skips privatisation of all
linear variables during the FIR generation phase, since linear variables
are handled in their entirety by the OpenMP IRBuilder. However,
"implicit" linear variables (like OmpPreDetermined) cannot be skipped,
since FIR generation requires privatized symbols. This patch adds checks
to skip the same.


Fixes https://github.com/llvm/llvm-project/issues/142935

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 1b8670b379f82..3fae3f3a0ddfd 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -204,6 +204,42 @@ void DataSharingProcessor::collectOmpObjectListSymbol(
 }
 
 void DataSharingProcessor::collectSymbolsForPrivatization() {
+  // Add checks here for exceptional cases where privatization is not
+  // needed and be deferred to a later phase (like OpenMP IRBuilder).
+  // Such cases are suggested to be clearly documented and explained
+  // instead of being silently skipped
+  auto isException = [&](const Fortran::semantics::Symbol *sym) -> bool {
+    // `OmpPreDetermined` symbols cannot be exceptions since
+    // their privatized symbols are heavily used in FIR.
+    if (sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined))
+      return false;
+
+    // The handling of linear clause is deferred to the OpenMP
+    // IRBuilder which is responsible for all its aspects,
+    // including privatization. Privatizing linear variables at this point would
+    // cause the following structure:
+    //
+    // omp.op linear(%linear = %step : !fir.ref<type>) {
+    //	Use %linear in this BB
+    // }
+    //
+    // to be changed to the following:
+    //
+    // omp. op linear(%linear = %step : !fir.ref<type>)
+    // 	private(%linear -> %arg0 : !fir.ref<i32>) {
+    //	Declare and use %arg0 in this BB
+    // }
+    //
+    // The OpenMP IRBuilder needs to map the linear MLIR value
+    // (i.e. %linear) to its `uses` in the BB to correctly
+    // implement the functionalities of linear clause. However,
+    // privatizing here disallows the IRBuilder to
+    // draw a relation between %linear and %arg0. Hence skip.
+    if (sym->test(Fortran::semantics::Symbol::Flag::OmpLinear))
+      return true;
+    return false;
+  };
+
   for (const omp::Clause &clause : clauses) {
     if (const auto &privateClause =
             std::get_if<omp::clause::Private>(&clause.u)) {
@@ -222,10 +258,10 @@ void DataSharingProcessor::collectSymbolsForPrivatization() {
   }
 
   // TODO For common blocks, add the underlying objects within the block. Doing
-  // so, we won't need to explicitely handle block objects (or forget to do
+  // so, we won't need to explicitly handle block objects (or forget to do
   // so).
   for (auto *sym : explicitlyPrivatizedSymbols)
-    if (!sym->test(Fortran::semantics::Symbol::Flag::OmpLinear))
+    if (!isException(sym))
       allPrivatizedSymbols.insert(sym);
 }