[clang] [flang] [llvm] [clang][flang][OpenMP] Fix context selector matching and scoring (PR #224431)

[via cfe-commits]

================
@@ -7743,15 +7801,26 @@ static void genMetadirective(lower::AbstractConverter &converter,
       TODO(variantLoc,
            "METADIRECTIVE with both block- and loop-associated variants");
 
-    genOMPDispatch(converter, symTable, semaCtx, eval, variantLoc, queue,
-                   queue.begin());
+    if (consumesBody) {
+      if (associatedBlockEval && associatedBlockEval->lowerAsUnstructured())
+        TODO(variantLoc,
+             "unstructured associated BLOCK in METADIRECTIVE variant");
+      mlir::SaveStateStack<OpenMPContextFrame> context{
+          converter.getStateStack(), eval, spec->DirId(),
+          /*isReplacement=*/true};
+      genOMPDispatch(converter, symTable, semaCtx, eval, variantLoc, queue,
----------------
MattPD wrote:

In a BLOCK associated with a selected PARALLEL, a sequential DO loop still uses the outer storage of its DO variable. Under [OpenMP 5.2 5.1.1](https://www.openmp.org/spec-html/5.2/openmpsu33.html), a sequential DO variable inside PARALLEL is private. A directly written PARALLEL privatizes the variable, but the selected PARALLEL does not.

You can reproduce this by saving the following to `repro.f90`, building it with `flang -fopenmp -fopenmp-version=52 -module-dir /tmp repro.f90 -o repro`, and running `./repro`:

```fortran
subroutine s()
  integer :: i
  i = -7
  !$omp metadirective when(implementation={vendor(llvm)}: parallel num_threads(1))
  block
    do i = 1, 1
      call observe(i)
    end do
  end block
  print *, i
end subroutine
subroutine observe(i)
  integer :: i
end subroutine
program p
  call s()
end program
```

At 1850727, `./repro` prints `2` instead of `-7`. A separate two-thread address experiment shows that the selected PARALLEL gives both threads the same storage for `i`, while a directly written PARALLEL gives each thread its own. At 9d79b83 and at the merge base, Flang omits the selected PARALLEL, so only one thread runs the loop. Associating the BLOCK with the selected PARALLEL therefore exposes the missing privatization to concurrent execution.

The same missing data-sharing analysis affects `parallel default(firstprivate)` and implicit firstprivate captures in TASK. Could lowering establish the BLOCK's symbols, attributes, and name bindings separately for each candidate? Could lowering diagnose unsupported data-sharing cases before it emits the data environment? Checking only explicit data-sharing clauses would miss this case, because the reproducer has none.

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