Skip to content

[Flang][OpenMP]Make Do concurrent indices private #93785

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
Jul 2, 2024

Conversation

harishch4
Copy link
Contributor

Fixes: #85538

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics labels May 30, 2024
@llvmbot
Copy link
Member

llvmbot commented May 30, 2024

@llvm/pr-subscribers-flang-semantics

@llvm/pr-subscribers-flang-openmp

Author: None (harishch4)

Changes

Fixes: #85538


Full diff: https://github.com/llvm/llvm-project/pull/93785.diff

2 Files Affected:

  • (modified) flang/lib/Semantics/resolve-directives.cpp (+22-3)
  • (added) flang/test/Semantics/OpenMP/doconcurrent01.f90 (+17)
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index dbc531372c3f4..3992d527c7ae5 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1698,10 +1698,10 @@ 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
 bool OmpAttributeVisitor::Pre(const parser::DoConstruct &x) {
-  // TODO:[OpenMP 5.1] DO CONCURRENT indices are private
-  if (x.IsDoNormal()) {
-    if (!dirContext_.empty() && GetContext().withinConstruct) {
+  if (!dirContext_.empty() && GetContext().withinConstruct) {
+    if (x.IsDoNormal()) {
       const parser::Name *iv{GetLoopIndex(x)};
       if (iv && iv->symbol) {
         if (!iv->symbol->test(Symbol::Flag::OmpPreDetermined)) {
@@ -1721,6 +1721,25 @@ bool OmpAttributeVisitor::Pre(const parser::DoConstruct &x) {
           }
         }
       }
+    } 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) {
+          if (!iv->symbol->test(Symbol::Flag::OmpPreDetermined)) {
+            ResolveSeqLoopIndexInParallelOrTaskConstruct(*iv);
+          } else {
+            // TODO: conflict checks with explicitly determined DSA
+          }
+        }
+      }
     }
   }
   return true;
diff --git a/flang/test/Semantics/OpenMP/doconcurrent01.f90 b/flang/test/Semantics/OpenMP/doconcurrent01.f90
new file mode 100644
index 0000000000000..7e3bdce871dd4
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/doconcurrent01.f90
@@ -0,0 +1,17 @@
+! RUN: %python %S/../test_symbols.py %s %flang_fc1 -fopenmp
+
+! OpenMP 5.1.1
+! DO Concurrent indices are private
+
+!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)
+   do concurrent(i=1:2)
+   end do
+   !$omp end single
+   !$omp end parallel
+end subroutine

Comment on lines 1724 to 1742
} 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) {
if (!iv->symbol->test(Symbol::Flag::OmpPreDetermined)) {
ResolveSeqLoopIndexInParallelOrTaskConstruct(*iv);
} else {
// TODO: conflict checks with explicitly determined DSA
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the threadprivate check not applicable here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The loop iteration variable may not appear in a threadprivatedirective. Yeah, sorry I missed it. If it's okay, I'll store symbols in a list and move common code out.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Due to a new symbol being declared here, the check won't work for DoConcurrent. I'll create an issue to try to handle it in a separate patch.

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

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

LG.

@harishch4 harishch4 merged commit 471ca94 into llvm:main Jul 2, 2024
7 checks passed
lravenclaw pushed a commit to lravenclaw/llvm-project that referenced this pull request Jul 3, 2024
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:openmp flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Flang][OpenMP] LLVM ERROR: symbol not mapped
3 participants