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

[Leandro Lupori via flang-commits]

================
@@ -268,21 +268,39 @@ void DataSharingProcessor::insertLastPrivateCompare(mlir::Operation *op) {
   firOpBuilder.restoreInsertionPoint(localInsPt);
 }
 
+void DataSharingProcessor::collectSymbolsInNestedRegions(
+    Fortran::lower::pft::Evaluation &eval,
+    Fortran::semantics::Symbol::Flag flag,
+    llvm::SetVector<const Fortran::semantics::Symbol *>
+        &symbolsInNestedRegions) {
+  for (Fortran::lower::pft::Evaluation &nestedEval :
+       eval.getNestedEvaluations()) {
+    if (nestedEval.hasNestedEvaluations()) {
+      if (nestedEval.isConstruct())
+        // Recursively look for OpenMP constructs within `nestedEval`'s region
+        collectSymbolsInNestedRegions(nestedEval, flag, symbolsInNestedRegions);
+      else
+        converter.collectSymbolSet(nestedEval, symbolsInNestedRegions, flag,
+                                   /*collectSymbols=*/true,
+                                   /*collectHostAssociatedSymbols=*/false);
+    }
+  }
+}
+
+// Collect symbols to be default privatized in two steps.
+// In step 1, collect all symbols in `eval` that match `flag`
+// into `defaultSymbols`.
+// In step 2, for nested constructs (if any), if and only if
+// the nested construct is an OpenMP construct, collect those nested
+// symbols skipping host assicauted symbols into `symbolsInNestedRegions`.
+// Finally, in current context, lower all symbols in the set
+// `defaultSymbols` - `symbolsInNestedRegions`.
----------------
luporl wrote:

Without reading `collectSymbols` code, this may leave the impression that this member function also does the lowering.

Maybe it could be changed to something like:
Later, in current context, all symbols in the set `defaultSymbols` - `symbolsInNestedRegions` will be privatized.

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