diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp index 67a9a4675f115..49d8b46beb0eb 100644 --- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp +++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp @@ -36,12 +36,12 @@ namespace lower { namespace omp { bool DataSharingProcessor::OMPConstructSymbolVisitor::isSymbolDefineBy( const semantics::Symbol *symbol, lower::pft::Evaluation &eval) const { - return eval.visit( - common::visitors{[&](const parser::OpenMPConstruct &functionParserNode) { - return symDefMap.count(symbol) && - symDefMap.at(symbol) == &functionParserNode; - }, - [](const auto &functionParserNode) { return false; }}); + return eval.visit(common::visitors{ + [&](const parser::OpenMPConstruct &functionParserNode) { + return symDefMap.count(symbol) && + symDefMap.at(symbol) == ConstructPtr(&functionParserNode); + }, + [](const auto &functionParserNode) { return false; }}); } static bool isConstructWithTopLevelTarget(lower::pft::Evaluation &eval) { @@ -553,8 +553,16 @@ void DataSharingProcessor::collectSymbols( return sym->test(semantics::Symbol::Flag::OmpImplicit); } - if (collectPreDetermined) - return sym->test(semantics::Symbol::Flag::OmpPreDetermined); + 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 + // 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) && + sym->test(semantics::Symbol::Flag::OmpPreDetermined); + } return !sym->test(semantics::Symbol::Flag::OmpImplicit) && !sym->test(semantics::Symbol::Flag::OmpPreDetermined); diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.h b/flang/lib/Lower/OpenMP/DataSharingProcessor.h index 96e7fa6d5d93c..335699577ea84 100644 --- a/flang/lib/Lower/OpenMP/DataSharingProcessor.h +++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.h @@ -19,6 +19,7 @@ #include "flang/Parser/parse-tree.h" #include "flang/Semantics/symbol.h" #include "mlir/Dialect/OpenMP/OpenMPDialect.h" +#include namespace mlir { namespace omp { @@ -58,13 +59,18 @@ class DataSharingProcessor { } void Post(const parser::Name &name) { - auto *current = !constructs.empty() ? constructs.back() : nullptr; + auto current = !constructs.empty() ? constructs.back() : ConstructPtr(); symDefMap.try_emplace(name.symbol, current); } - llvm::SmallVector constructs; - llvm::DenseMap - symDefMap; + bool Pre(const parser::DeclarationConstruct &decl) { + constructs.push_back(&decl); + return true; + } + + void Post(const parser::DeclarationConstruct &decl) { + constructs.pop_back(); + } /// Given a \p symbol and an \p eval, returns true if eval is the OMP /// construct that defines symbol. @@ -72,6 +78,11 @@ class DataSharingProcessor { lower::pft::Evaluation &eval) const; private: + using ConstructPtr = std::variant; + llvm::SmallVector constructs; + llvm::DenseMap symDefMap; + unsigned version; }; diff --git a/flang/test/Lower/OpenMP/block_predetermined_privatization.f90 b/flang/test/Lower/OpenMP/block_predetermined_privatization.f90 new file mode 100644 index 0000000000000..12346c14ef5d9 --- /dev/null +++ b/flang/test/Lower/OpenMP/block_predetermined_privatization.f90 @@ -0,0 +1,32 @@ +! Fixes a bug when a block variable is marked as pre-determined 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_predetermined_privatization + implicit none + integer :: i + + !$omp parallel + do i=1,10 + block + integer :: j + do j=1,10 + end do + end block + end do + !$omp end parallel +end subroutine + +! CHECK-LABEL: func.func @_QPblock_predetermined_privatization() { +! CHECK: %[[I_DECL:.*]]:2 = hlfir.declare %{{.*}} {uniq_name = "{{.*}}Ei"} +! CHECK: omp.parallel private(@{{.*}}Ei_private_i32 %[[I_DECL]]#0 -> %{{.*}} : !fir.ref) { +! CHECK: fir.do_loop {{.*}} { +! Verify that `j` is allocated whithin the same scope of its block (i.e. inside +! the `parallel` loop). +! CHECK: fir.alloca i32 {bindc_name = "j", {{.*}}} +! CHECK: } +! CHECK: } +! CHECK: }