[flang-commits] [flang] [flang][OpenMP] Try to unify induction var privatization for sequential loops inside parallel regions. (PR #91116)

[Leandro Lupori via flang-commits]

================
@@ -344,7 +353,6 @@ void DataSharingProcessor::collectSymbols(
     assert(curScope && "couldn't find current scope");
     if (isPrivatizable(*sym) && !symbolsInNestedRegions.contains(sym) &&
         !privatizedSymbols.contains(sym) &&
-        !sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined) &&
----------------
luporl wrote:

> > Finally, in the non-delayed privatization path, pre-determined symbols were already being skipped:
> 
> Indeed. And this is what I am trying to prevent from happening here both for early and delayed privatization. AFAIU, I don't think there is a reason to have 2 different code-gen paths for pre-determined vs. non-pre-determined variables. Please let me know if I am mistaken here.
> 

I also don't think there is a reason to have 2 different paths to perform the privatization of pre-determined vs. non-predetermined variables. However, currently `DataSharingProcessor::collectSymbols` is intended to collect default and implicit symbols only. It won't even be called if there are no default or implicit symbols.

> More context for this is in this Slack thread: https://flang-compiler.slack.com/archives/C01PY03PP9P/p1714729085154889.
> 
> > if you remove default(private), are induction variables correctly privatized?
> 
> In that case, they are not privatized at all in the sense that their alloca's are emitted simply as part of emitting the code for the do concurrent for which they are used as induction variables. They are not marked as pre-determined (see [this](https://flang-compiler.slack.com/archives/C01PY03PP9P/p1714899066421309?thread_ts=1714729085.154889&cid=C01PY03PP9P)).

Thanks for pointing this out. I think this is the main problem. Pre-determined (private) variables must be privatized independently of the presence of `private` or `default` clauses. So there is something wrong with case 2, as the loop variable should also be marked as pre-determined in semantics phase.

But actually, it seems that currently loop variables are always being assumed to be pre-determined, which, AFAIU, is not always the case (see test/Semantics/OpenMP/symbol08.f90, for instance).

Right now it would probably be easier to refactor `genLoopVars`, to somehow pass the loop variables to the DataSharingProcessor, so that privatization happens only there.

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