[flang-commits] [flang] [flang][OpenMP] Lower DO and SIMD variants in metadirectives (PR #210810)

[via flang-commits]

================
@@ -6054,50 +6284,80 @@ static void genMetadirective(lower::AbstractConverter &converter,
       TODO(variantLoc, "declarative METADIRECTIVE variant");
     }
 
+    bool hasLoopAssociation =
+        hasDirectiveAssociation(queue, llvm::omp::Association::LoopNest);
+    if (hasLoopAssociation) {
+      // Name resolution cannot give a metadirective variant its own DSA
+      // scope, so marking its loop IV can otherwise contaminate an enclosing
+      // data environment.
+      if (isNestedInOpenMPDataEnvironment(
+              eval, builder.getInsertionBlock()->getParentOp()))
+        TODO(variantLoc, "loop-associated METADIRECTIVE nested in an OpenMP "
+                         "data environment");
+      if (hasUnsupportedDataEnvironmentDirective(queue))
----------------
MattPD wrote:

This guard and its neighbors all sit inside `if (hasLoopAssociation)`. A block-associated variant reaches `genOMPDispatch` without an equivalent check, and it then misses the data-sharing attributes that direct lowering provides.

```fortran
subroutine base_par(n, a)
  integer :: n, a(n), i
  !$omp parallel
  do i = 1, n
    a(i) = i
  end do
  !$omp end parallel
end subroutine

subroutine meta_par(n, a)
  integer :: n, a(n), i
  !$omp begin metadirective when(implementation={vendor(llvm)}: parallel) otherwise(nothing)
  do i = 1, n
    a(i) = i
  end do
  !$omp end metadirective
end subroutine
```

`base_par` privatizes the induction variable, and `meta_par` does not:

```
omp.parallel private(@_QFbase_parEi_private_i32 %3#0 -> %arg2 : !fir.ref<i32>) {
omp.parallel {
```

The `meta_par` loop body stores to the outer `i`. Under `-mmlir --enable-delayed-privatization=false`, `base_par` allocates a thread-local `fir.alloca i32 {bindc_name = "i", pinned, uniq_name = "_QFbase_parEi"}` inside the region. `meta_par` allocates none. Every thread therefore shares `i` in both privatization modes.

`task` loses its attributes the same way. Written directly it emits `omp.task private(@_QFbase_taskEx_firstprivate_i32 ...)`, and under a metadirective it emits a bare `omp.task {`.

This path predates the patch, so the answer may be that it is out of scope. Is the block path deliberately exempt from the data-environment restriction the loop path enforces? If it is not, running `hasUnsupportedDataEnvironmentDirective` before the fall-through dispatch would turn silent wrong code into a TODO.

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