Skip to content

[flang][openacc] Do not accept static and num for gang clause on routine dir #77673

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 4 commits into from
Jan 11, 2024

Conversation

clementval
Copy link
Contributor

Only the dim argument is allowed on the gang clause for the routine directive. Reject static and num arguments in the semantic check.

@llvmbot llvmbot added flang Flang issues not falling into any other category openacc flang:semantics labels Jan 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 10, 2024

@llvm/pr-subscribers-mlir-openacc
@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-openacc

@llvm/pr-subscribers-flang-semantics

Author: Valentin Clement (バレンタイン クレメン) (clementval)

Changes

Only the dim argument is allowed on the gang clause for the routine directive. Reject static and num arguments in the semantic check.


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

2 Files Affected:

  • (modified) flang/lib/Semantics/check-acc-structure.cpp (+12)
  • (modified) flang/test/Semantics/OpenACC/acc-routine.f90 (+10)
diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index 4a5798a8a531a4..5c2a871c322e3a 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -560,14 +560,26 @@ void AccStructureChecker::Enter(const parser::AccClause::Gang &g) {
   if (g.v) {
     bool hasNum = false;
     bool hasDim = false;
+    bool hasStatic = false;
     const Fortran::parser::AccGangArgList &x = *g.v;
     for (const Fortran::parser::AccGangArg &gangArg : x.v) {
       if (std::get_if<Fortran::parser::AccGangArg::Num>(&gangArg.u))
         hasNum = true;
       else if (std::get_if<Fortran::parser::AccGangArg::Dim>(&gangArg.u))
         hasDim = true;
+      else if (std::get_if<Fortran::parser::AccGangArg::Static>(&gangArg.u))
+        hasStatic = true;
     }
 
+    if (GetContext().directive == llvm::acc::Directive::ACCD_routine &&
+        (hasStatic || hasNum))
+      context_.Say(GetContext().clauseSource,
+          "Only the dim argument is allowed on the %s clause on the %s directive"_err_en_US,
+          parser::ToUpperCaseLetters(
+              llvm::acc::getOpenACCClauseName(llvm::acc::Clause::ACCC_gang)
+                  .str()),
+          ContextDirectiveAsFortran());
+
     if (hasDim && hasNum)
       context_.Say(GetContext().clauseSource,
           "The num argument is not allowed when dim is specified"_err_en_US);
diff --git a/flang/test/Semantics/OpenACC/acc-routine.f90 b/flang/test/Semantics/OpenACC/acc-routine.f90
index 4dcb849c642c83..f27084115fbee2 100644
--- a/flang/test/Semantics/OpenACC/acc-routine.f90
+++ b/flang/test/Semantics/OpenACC/acc-routine.f90
@@ -13,3 +13,13 @@ subroutine sub2(a)
 subroutine sub3()
   !$acc routine bind(sub1)
 end subroutine
+
+subroutine sub4()
+  !ERROR: Only the dim argument is allowed on the GANG clause on the ROUTINE directive
+  !$acc routine gang(num: 1)
+end subroutine
+
+subroutine sub5()
+  !ERROR: Only the dim argument is allowed on the GANG clause on the ROUTINE directive
+  !$acc routine gang(static: 1)
+end subroutine

@clementval clementval requested a review from jeanPerier January 11, 2024 17:14
Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

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

Nit, LGTM otherwise.

const Fortran::parser::AccGangArgList &x = *g.v;
for (const Fortran::parser::AccGangArg &gangArg : x.v) {
if (std::get_if<Fortran::parser::AccGangArg::Num>(&gangArg.u))
hasNum = true;
else if (std::get_if<Fortran::parser::AccGangArg::Dim>(&gangArg.u))
hasDim = true;
else if (std::get_if<Fortran::parser::AccGangArg::Static>(&gangArg.u))
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing braces given this is in semantics.

Copy link
Contributor

@razvanlupusoru razvanlupusoru left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good to me.

Copy link

github-actions bot commented Jan 11, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@clementval clementval force-pushed the acc_routine_gang_sema branch from ac3743c to 0ac594f Compare January 11, 2024 21:14
@clementval clementval merged commit 238b579 into llvm:main Jan 11, 2024
@clementval clementval deleted the acc_routine_gang_sema branch January 11, 2024 21:42
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…ine dir (llvm#77673)

Only the dim argument is allowed on the gang clause for the routine
directive. Reject static and num arguments in the semantic check.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:semantics flang Flang issues not falling into any other category mlir:openacc mlir openacc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants