[flang-commits] [flang] [flang][OpenMP] Fix crash when a sliced array is specified in a forall within a workshare construct (PR #170913)

[Tom Eccles via flang-commits]

================
@@ -136,8 +136,48 @@ static bool mustParallelizeOp(Operation *op) {
 }
 
 static bool isSafeToParallelize(Operation *op) {
-  return isa<hlfir::DeclareOp>(op) || isa<fir::DeclareOp>(op) ||
-         isMemoryEffectFree(op);
+  if (isa<hlfir::DeclareOp>(op) || isa<fir::DeclareOp>(op) ||
+      isMemoryEffectFree(op))
+    return true;
+
+  // Special case: Thread-local variables allocated in the OpenMP parallel
+  // region (using fir.alloca) are private to each thread and thus safe (and
+  // sometimes required) to parallelize. If the compiler wraps such load/stores
+  // in an omp.workshare block, only one thread updates its local copy, while
+  // all other threads read uninitialized data (see issue #143330).
+  if (fir::StoreOp store = dyn_cast<fir::StoreOp>(op)) {
----------------
tblah wrote:

1. Please refactor this so that the same body can be shared for both load and store. Maybe we need a generic helper for deciding if something is an openmp threadlocal variable (document that the analysis is best effort e.g. we don't know that we are inside of a parallel region if the code is inside a function called from the parallel region).
2. You can use fir::AliasAnalysis to get the "source" of a memory reference. This understands other cases than just (hl)fir.declare (it can do converts, reboxes, etc).
3. Another common case you might want to consider are variables who's memory comes appears to come from an openmp clause. Some clauses (e.g. private, firstprivate, lastprivate, reduction, linear) create threadlocal memory.

https://github.com/llvm/llvm-project/pull/170913