[flang-commits] [flang] [flang][OpenMP] Fix construct privatization in default clause (PR #72510)

[Leandro Lupori via flang-commits]

luporl wrote:

Hi @NimishMishra,

> I am revisiting this patch, and am wondering if recursion is needed here. Checking up with you once if I am missing something.
> 
> ```
> !$omp parallel do default(private) firstprivate(y)
>   do i = 1, 10
>     y = 1           ! Point X
>     !$omp parallel default(private)
>       y = 2        ! Point Y
>     !$omp end parallel
>   end do
> !$omp end parallel do
> end
> ```
> 
> Then `collectSymbolSet()` on the outermost evaluation shall collect all symbols within the region. Then we enter into its nested evaluations and catch `do{... region ...}`, which is essentially,

Right. IIUC, `do{... region ...}` is the only nested evaluation in this case.

> 
> ```
> do{
>      assignment-stmt
>      parallel{... region ...}
> }
> ```
> 
> So if we iterate over these evaluations, and do the following:
> 
>     1. For non-OpenMP constructs, skip adding symbols to `symbolsInNestedRegion`. This shall skip the symbol `y` marked at `Point X`.
> 
>     2. For OpenMP constructs, add symbols. This shall add the symbol `y` marked at `Point Y`.
> 
> 
> Concisely, for a `do{... region ...}`, if we have a OpenMP region inside it, then we can add **all** symbols within it to `symbolsInNestedRegion` with just one `collectSymbolSet` call (which in turn is recursive). So I was wondering if we need additional recursion at this point.

Let me see if I understood it correctly. Are you proposing to add an extra loop to iterate over the nested evaluations of each "top" nested evaluation, applying rules 1 and 2 to each of them?
If so, this should fix this test program, but not others with deeper nesting, such as:

```f90
!$omp parallel do default(private) firstprivate(y)
  do i = 1, 10
    y = 1
    do j = 1, 10
      !$omp parallel default(private)
        y = 2
      !$omp end parallel
    end do
  end do
!$omp end parallel do
end
```

That's why I think that, at least with this approach, recursion would be needed to go through the nested evaluations properly. I agree that whenever an OpenMP construct is found, one call to `collectSymbolSet` is enough to collect all symbols in it and no further recursion is needed in that branch. But for non-OpenMP constructs, I don't see how to find if there is a nested OpenMP construct in them without scanning each nested evaluation recursively. I hope this won't be too costly in large programs.

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