[Flang][OpenMP] Don't expect block arguments using early privatization

[skatrak]

There are some spots where all symbols to privatize collected by a DataSharingProcessor instance are expected to have corresponding entry block arguments associated regardless of whether delayed privatization was enabled.

This can result in compiler crashes if a DataSharingProcessor instance created with useDelayedPrivatization=false is queried in this way. The solution proposed by this patch is to provide another public method to query specifically delayed privatization symbols, which will either be empty or point to the complete set of symbols to privatize accordingly.

[llvmbot]

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

Author: Sergio Afonso (skatrak)

Changes

There are some spots where all symbols to privatize collected by a DataSharingProcessor instance are expected to have corresponding entry block arguments associated regardless of whether delayed privatization was enabled.

This can result in compiler crashes if a DataSharingProcessor instance created with useDelayedPrivatization=false is queried in this way. The solution proposed by this patch is to provide another public method to query specifically delayed privatization symbols, which will either be empty or point to the complete set of symbols to privatize accordingly.

---

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

2 Files Affected:

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

diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.h b/flang/lib/Lower/OpenMP/DataSharingProcessor.h
index 1e9d07ec13857d..166cdfa37fc7fa 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.h
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.h
@@ -153,6 +153,12 @@ class DataSharingProcessor {
   getAllSymbolsToPrivatize() const {
     return allPrivatizedSymbols;
   }
+
+  llvm::ArrayRef<const semantics::Symbol *> getDelayedPrivSymbols() const {
+    return useDelayedPrivatization
+               ? allPrivatizedSymbols.getArrayRef()
+               : llvm::ArrayRef<const semantics::Symbol *>();
+  }
 };
 
 } // namespace omp
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index d614db8b68ef65..e445731883f06f 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -882,7 +882,7 @@ static void genBodyOfTargetOp(
   }
 
   for (auto [argIndex, argSymbol] :
-       llvm::enumerate(dsp.getAllSymbolsToPrivatize())) {
+       llvm::enumerate(dsp.getDelayedPrivSymbols())) {
     argIndex = mapSyms.size() + argIndex;
 
     const mlir::BlockArgument &arg = region.getArgument(argIndex);
@@ -1494,8 +1494,8 @@ genParallelOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
                              allRegionArgLocs);
 
     llvm::SmallVector<const semantics::Symbol *> allSymbols(reductionSyms);
-    allSymbols.append(dsp.getAllSymbolsToPrivatize().begin(),
-                      dsp.getAllSymbolsToPrivatize().end());
+    allSymbols.append(dsp.getDelayedPrivSymbols().begin(),
+                      dsp.getDelayedPrivSymbols().end());
 
     unsigned argIdx = 0;
     for (const semantics::Symbol *arg : allSymbols) {

[tblah]

LGTM, thanks!