[flang-commits] [flang] [flang][OpenMP] Fix wrong results for FORALL in a workshare construct (PR #211371)

[Tom Eccles via flang-commits]

================
@@ -328,9 +359,19 @@ static void collectThreadLocalWrites(Operation *op,
 // Reads are matched by their underlying thread-local allocation, mirroring
 // collectThreadLocalWrites, so that a load through a fir.declare/fir.convert
 // still keeps the corresponding write live for broadcasting.
-static void collectThreadLocalReads(Region &scope,
-                                    llvm::SmallDenseSet<Value> &reads) {
+//
+// An opaque call may read any thread-local location inside its callee, and a
+// memory-effecting operation may report a read of unspecified memory (a read
+// effect with no attached value). Neither read can be attributed to a specific
+// location, so reads.unknown is set to force every thread-local write to be
+// broadcasted. Other interface-less operations (e.g. omp.barrier, fir.declare)
+// have known, inspectable behaviour and are safe to ignore here.
+static void collectThreadLocalReads(Region &scope, ThreadLocalReads &reads) {
   scope.walk([&](Operation *op) {
+    if (isa<mlir::CallOpInterface>(op)) {
+      reads.unknown = true;
+      return;
+    }
     auto memEffects = dyn_cast<MemoryEffectOpInterface>(op);
     if (!memEffects)
       return;
----------------
tblah wrote:

Please add a comment with a TODO explaining why this doesn't set reads.unknown.

Really this needs to be fixed in the dialects if possible, although that is out of scope for this patch. Do you want to do this in follow up patches or should I?

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