Skip to content

[flang] Extend omp loop semantic checks for reduction #128823

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 1 commit into from
Feb 27, 2025

Conversation

ergawy
Copy link
Member

@ergawy ergawy commented Feb 26, 2025

Extend semantic checks for omp loop directive to report errors when a reduction clause is specified on a standalone loop directive with teams binding.

This is similar to how clang behaves.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:openmp flang:semantics labels Feb 26, 2025
@ergawy ergawy requested review from mjklemm and kparzysz February 26, 2025 05:50
@llvmbot
Copy link
Member

llvmbot commented Feb 26, 2025

@llvm/pr-subscribers-flang-openmp

Author: Kareem Ergawy (ergawy)

Changes

Extend semantic checks for omp loop directive to report errors when a reduction clause is specified on a standalone loop directive with teams binding.

This is similar to how clang behaves.


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

2 Files Affected:

  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+12)
  • (modified) flang/test/Semantics/OpenMP/loop-bind.f90 (+5)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index ef7204dcd9196..e6e36b773b3c5 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3118,6 +3118,18 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Reduction &x) {
   if (llvm::omp::nestedReduceWorkshareAllowedSet.test(GetContext().directive)) {
     CheckSharedBindingInOuterContext(objects);
   }
+
+  if (GetContext().directive == llvm::omp::Directive::OMPD_loop) {
+    for (auto clause : GetContext().clauseInfo) {
+      if (const auto *bindClause{
+              std::get_if<parser::OmpClause::Bind>(&clause.second->u)}) {
+        if (bindClause->v.v == parser::OmpBindClause::Binding::Teams) {
+          context_.Say(GetContext().clauseSource,
+              "'REDUCTION' clause not allowed with '!$OMP LOOP BIND(TEAMS)."_err_en_US);
+        }
+      }
+    }
+  }
 }
 
 void OmpStructureChecker::Enter(const parser::OmpClause::InReduction &x) {
diff --git a/flang/test/Semantics/OpenMP/loop-bind.f90 b/flang/test/Semantics/OpenMP/loop-bind.f90
index f3aa9d19fe989..8f03910644370 100644
--- a/flang/test/Semantics/OpenMP/loop-bind.f90
+++ b/flang/test/Semantics/OpenMP/loop-bind.f90
@@ -30,4 +30,9 @@ program main
   end do
   !$omp end teams loop
 
+  !ERROR: 'REDUCTION' clause not allowed with '!$OMP LOOP BIND(TEAMS).
+  !$omp loop bind(teams) reduction(+: x)
+  do i = 0, 10
+    x = x + i
+  end do
 end program main

@llvmbot
Copy link
Member

llvmbot commented Feb 26, 2025

@llvm/pr-subscribers-flang-semantics

Author: Kareem Ergawy (ergawy)

Changes

Extend semantic checks for omp loop directive to report errors when a reduction clause is specified on a standalone loop directive with teams binding.

This is similar to how clang behaves.


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

2 Files Affected:

  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+12)
  • (modified) flang/test/Semantics/OpenMP/loop-bind.f90 (+5)
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index ef7204dcd9196..e6e36b773b3c5 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3118,6 +3118,18 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Reduction &x) {
   if (llvm::omp::nestedReduceWorkshareAllowedSet.test(GetContext().directive)) {
     CheckSharedBindingInOuterContext(objects);
   }
+
+  if (GetContext().directive == llvm::omp::Directive::OMPD_loop) {
+    for (auto clause : GetContext().clauseInfo) {
+      if (const auto *bindClause{
+              std::get_if<parser::OmpClause::Bind>(&clause.second->u)}) {
+        if (bindClause->v.v == parser::OmpBindClause::Binding::Teams) {
+          context_.Say(GetContext().clauseSource,
+              "'REDUCTION' clause not allowed with '!$OMP LOOP BIND(TEAMS)."_err_en_US);
+        }
+      }
+    }
+  }
 }
 
 void OmpStructureChecker::Enter(const parser::OmpClause::InReduction &x) {
diff --git a/flang/test/Semantics/OpenMP/loop-bind.f90 b/flang/test/Semantics/OpenMP/loop-bind.f90
index f3aa9d19fe989..8f03910644370 100644
--- a/flang/test/Semantics/OpenMP/loop-bind.f90
+++ b/flang/test/Semantics/OpenMP/loop-bind.f90
@@ -30,4 +30,9 @@ program main
   end do
   !$omp end teams loop
 
+  !ERROR: 'REDUCTION' clause not allowed with '!$OMP LOOP BIND(TEAMS).
+  !$omp loop bind(teams) reduction(+: x)
+  do i = 0, 10
+    x = x + i
+  end do
 end program main

@ergawy
Copy link
Member Author

ergawy commented Feb 27, 2025

Ping! Please take a look when you have time.

Copy link
Contributor

@mjklemm mjklemm left a comment

Choose a reason for hiding this comment

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

LGTM, except the two nits about missing an ' sign in the error message.

Extend semantic checks for `omp loop` directive to report errors when
a `reduction` clause is specified on a standalone `loop` directive with
`teams` binding.

This is similar to how clang behaves.
@ergawy ergawy force-pushed the users/ergawy/loop_reduction_1 branch from 8f2c57e to 5b88e7d Compare February 27, 2025 10:21
@ergawy ergawy merged commit f6262fa into main Feb 27, 2025
11 checks passed
@ergawy ergawy deleted the users/ergawy/loop_reduction_1 branch February 27, 2025 11:08
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Mar 3, 2025
Extend semantic checks for `omp loop` directive to report errors when a
`reduction` clause is specified on a standalone `loop` directive with
`teams` binding.

This is similar to how clang behaves.
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.

3 participants