Skip to content

[flang][OpenMP] Don't try to privatize FORALL/DO CONCURRENT indices #123341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 1 addition & 18 deletions flang/lib/Semantics/resolve-directives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1777,28 +1777,13 @@ void OmpAttributeVisitor::ResolveSeqLoopIndexInParallelOrTaskConstruct(
// Use of DO CONCURRENT inside OpenMP construct is unspecified behavior
// till OpenMP-5.0 standard.
// In above both cases we skip the privatization of iteration variables.
// [OpenMP 5.1] DO CONCURRENT indices are private
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it would be better to include a comment like the one below to explain why nothing is done for DO CONCURRENT in this function:

// DO CONCURRENT indices have predetermined private DSA, but as they are
// defined in the construct itself, and OpenMP directives may not appear in it,
// they are already private.

bool OmpAttributeVisitor::Pre(const parser::DoConstruct &x) {
if (!dirContext_.empty() && GetContext().withinConstruct) {
llvm::SmallVector<const parser::Name *> ivs;
if (x.IsDoNormal()) {
const parser::Name *iv{GetLoopIndex(x)};
if (iv && iv->symbol)
ivs.push_back(iv);
} else if (x.IsDoConcurrent()) {
const Fortran::parser::LoopControl *loopControl = &*x.GetLoopControl();
const Fortran::parser::LoopControl::Concurrent &concurrent =
std::get<Fortran::parser::LoopControl::Concurrent>(loopControl->u);
const Fortran::parser::ConcurrentHeader &concurrentHeader =
std::get<Fortran::parser::ConcurrentHeader>(concurrent.t);
const std::list<Fortran::parser::ConcurrentControl> &controls =
std::get<std::list<Fortran::parser::ConcurrentControl>>(
concurrentHeader.t);
for (const auto &control : controls) {
const parser::Name *iv{&std::get<0>(control.t)};
if (iv && iv->symbol)
ivs.push_back(iv);
}
}
ordCollapseLevel--;
for (auto iv : ivs) {
Expand All @@ -1810,9 +1795,6 @@ bool OmpAttributeVisitor::Pre(const parser::DoConstruct &x) {
if (ordCollapseLevel) {
if (const auto *details{iv->symbol->detailsIf<HostAssocDetails>()}) {
const Symbol *tpSymbol = &details->symbol();
// TODO: DoConcurrent won't capture the following check because a new
// symbol is declared in ResolveIndexName(), which will not have the
// OmpThreadprivate flag.
if (tpSymbol->test(Symbol::Flag::OmpThreadprivate)) {
context_.Say(iv->source,
"Loop iteration variable %s is not allowed in THREADPRIVATE."_err_en_US,
Expand Down Expand Up @@ -2119,6 +2101,7 @@ static bool IsPrivatizable(const Symbol *sym) {
*sym) && /* OpenMP 5.2, 5.1.1: Assumed-size arrays are shared*/
!sym->owner().IsDerivedType() &&
sym->owner().kind() != Scope::Kind::ImpliedDos &&
sym->owner().kind() != Scope::Kind::Forall &&
!sym->detailsIf<semantics::AssocEntityDetails>() &&
!sym->detailsIf<semantics::NamelistDetails>() &&
(!misc ||
Expand Down
10 changes: 7 additions & 3 deletions flang/test/Semantics/OpenMP/doconcurrent01.f90
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
! RUN: %python %S/../test_symbols.py %s %flang_fc1 -fopenmp

! OpenMP 5.1.1
! DO Concurrent indices are private
! OpenMP 5.2 5.1.1 Variables Referenced in a Construct
! DO CONCURRENT indices have predetermined private DSA.
!
! As DO CONCURRENT indices are defined in the construct itself, and OpenMP
! directives may not appear in it, they are already private.
! Check that index symbols are not modified.

!DEF: /private_iv (Subroutine)Subprogram
subroutine private_iv
!DEF: /private_iv/i ObjectEntity INTEGER(4)
integer i
!$omp parallel default(private)
!$omp single
!DEF: /private_iv/OtherConstruct1/i (OmpPrivate, OmpPreDetermined) HostAssoc INTEGER(4)
!DEF: /private_iv/OtherConstruct1/OtherConstruct1/Forall1/i ObjectEntity INTEGER(4)
do concurrent(i=1:2)
end do
!$omp end single
Expand Down
32 changes: 32 additions & 0 deletions flang/test/Semantics/OpenMP/forall.f90
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
! RUN: %python %S/../test_symbols.py %s %flang_fc1 -fopenmp

! OpenMP 5.2 5.1.1 Variables Referenced in a Construct
! FORALL indices have predetermined private DSA.
!
! As FORALL indices are defined in the construct itself, and OpenMP
! directives may not appear in it, they are already private.
! Check that index symbols are not modified.

!DEF: /MainProgram1/a ObjectEntity INTEGER(4)
!DEF: /MainProgram1/b ObjectEntity INTEGER(4)
integer a(5), b(5)

!REF: /MainProgram1/a
a = 0
!REF: /MainProgram1/b
b = 0

!$omp parallel
!DEF: /MainProgram1/OtherConstruct1/Forall1/i (Implicit) ObjectEntity INTEGER(4)
!DEF: /MainProgram1/OtherConstruct1/a HostAssoc INTEGER(4)
!DEF: /MainProgram1/OtherConstruct1/b HostAssoc INTEGER(4)
forall(i = 1:5) a(i) = b(i) * 2
!$omp end parallel

!$omp parallel default(private)
!DEF: /MainProgram1/OtherConstruct2/Forall1/i (Implicit) ObjectEntity INTEGER(4)
!DEF: /MainProgram1/OtherConstruct2/a (OmpPrivate) HostAssoc INTEGER(4)
!DEF: /MainProgram1/OtherConstruct2/b (OmpPrivate) HostAssoc INTEGER(4)
forall(i = 1:5) a(i) = b(i) * 2
!$omp end parallel
end program
Loading