[flang][OpenMP] Don't privatize implicit symbols declare by nested BLOCKs

flang/lib/Lower/OpenMP/DataSharingProcessor.cpp

@@ -30,6 +30,7 @@
 #include "flang/Semantics/tools.h"
 #include "llvm/ADT/Sequence.h"
 #include "llvm/ADT/SmallSet.h"
+#include <variant>
 
 namespace Fortran {
 namespace lower {
@@ -44,6 +45,13 @@ bool DataSharingProcessor::OMPConstructSymbolVisitor::isSymbolDefineBy(
       [](const auto &functionParserNode) { return false; }});
 }
 
+bool DataSharingProcessor::OMPConstructSymbolVisitor::
+    isSymbolDefineByNestedDeclaration(const semantics::Symbol *symbol) const {
+  return symDefMap.count(symbol) &&
+         std::holds_alternative<const parser::DeclarationConstruct *>(
+             symDefMap.at(symbol));
+}
+
 static bool isConstructWithTopLevelTarget(lower::pft::Evaluation &eval) {
   const auto *ompEval = eval.getIf<parser::OpenMPConstruct>();
   if (ompEval) {
@@ -550,17 +558,20 @@ void DataSharingProcessor::collectSymbols(
         return false;
       }
 
-      return sym->test(semantics::Symbol::Flag::OmpImplicit);
-    }
-
-    if (collectPreDetermined) {
-      // Collect pre-determined symbols only if they are defined by the current
-      // OpenMP evaluation. If `sym` is not defined by the current OpenMP
+      // Collect implicit symbols only if they are not defined by a nested
+      // `DeclarationConstruct`. If `sym` is not defined by the current OpenMP
       // evaluation then it is defined by a block nested within the OpenMP
       // construct. This, in turn, means that the private allocation for the
       // symbol will be emitted as part of the nested block and there is no need
       // to privatize it within the OpenMP construct.
-      return visitor.isSymbolDefineBy(sym, eval) &&
+      return !visitor.isSymbolDefineByNestedDeclaration(sym) &&
+             sym->test(semantics::Symbol::Flag::OmpImplicit);
+    }
+
+    if (collectPreDetermined) {
+      // Similar to implicit symbols, collect pre-determined symbols only if
+      // they are not defined by a nested `DeclarationConstruct`
+      return !visitor.isSymbolDefineByNestedDeclaration(sym) &&
              sym->test(semantics::Symbol::Flag::OmpPreDetermined);
     }
 

flang/lib/Lower/OpenMP/DataSharingProcessor.h

@@ -77,6 +77,11 @@ class DataSharingProcessor {
     bool isSymbolDefineBy(const semantics::Symbol *symbol,
                           lower::pft::Evaluation &eval) const;
 
+    // Given a \p symbol, returns true if it is defined by a nested
+    // `DeclarationConstruct`.
+    bool
+    isSymbolDefineByNestedDeclaration(const semantics::Symbol *symbol) const;
+
   private:
     using ConstructPtr = std::variant<const parser::OpenMPConstruct *,
                                       const parser::DeclarationConstruct *>;

flang/test/Lower/OpenMP/block_implicit_privatization.f90

@@ -0,0 +1,31 @@
+! Fixes a bug when a block variable is marked as implicit private. In such
+! case, we can simply ignore privatizing that symbol within the context of the
+! currrent OpenMP construct since the "private" allocation for the symbol will
+! be emitted within the nested block anyway.
+
+! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
+
+subroutine block_implicit_privatization
+  implicit none
+  integer :: i
+
+  !$omp task
+  do i=1,10
+    block
+      integer :: j
+      j = 0
+    end block
+  end do
+  !$omp end task
+end subroutine
+
+! CHECK-LABEL: func.func @_QPblock_implicit_privatization() {
+! CHECK:         %[[I_DECL:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "{{.*}}Ei"}
+! CHECK:         omp.task private(@{{.*}}Ei_private_i32 %[[I_DECL]]#0 -> %{{.*}} : !fir.ref<i32>) {
+! CHECK:           fir.do_loop {{.*}} {
+! Verify that `j` is allocated whithin the same scope of its block (i.e. inside
+! the `task` loop).
+! CHECK:             fir.alloca i32 {bindc_name = "j", {{.*}}}
+! CHECK:           }
+! CHECK:         }
+! CHECK:       }