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

[Kiran Chandramohan via flang-commits]

================
@@ -426,14 +425,10 @@ void DataSharingProcessor::collectSymbols(
                              /*collectSymbols=*/true,
                              /*collectHostAssociatedSymbols=*/true);
   for (Fortran::lower::pft::Evaluation &e : eval.getNestedEvaluations()) {
-    if (e.hasNestedEvaluations())
+    if (e.hasNestedEvaluations() && !e.isConstruct())
----------------
kiranchandramohan wrote:

> So we maintain two lists: defaultSymbols and symbolsInNestedRegions. For non-constructs like OpenMP, we would not like to collected the host associated symbols for nested regions (thereby collectHostAssociatedSymbols = false). They can be handled as the default clause may be in their respective nested blocks. This was the earlier design.

Some OpenMP constructs (like critical) will not have a default clause because such a clause is not supported for these constructs. Some OpenMP constructs might not have them specified (i.e no default clause is present eventhough it is supported). Do we need to distinguish these cases? For those constructs which have them specified, for every pair of defaults do their exact values matter for the inner construct's symbols?

outer(default=shared), inner(default=shared) : Nothing to do
outer(default=private), inner(default=shared) : ?
outer(default=shared), inner(default=private) : ?
outer(default=private),inner(default=private) : ?

> However, for non-OpenMP constructs, the handling is different. This is because even though the inner construct has a block region (like OpenMP constructs do), all privatization of variables in the nested construct needs to happen in the outer OpenMP construct. Hence, we do not need to populate the symbolsInNestedRegions list.

Makes sense.


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