Skip to content

[flang][openacc] Make async clause behavior homogenous #136610

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 21, 2025
Merged

[flang][openacc] Make async clause behavior homogenous #136610

merged 1 commit into from
Apr 21, 2025

Conversation

clementval
Copy link
Contributor

The async clause was not handed in a similar way on serial, parallel and kernels directive. This patches updates the ACC.td file and the flang semantic to make it homogenous.

@llvmbot llvmbot added flang Flang issues not falling into any other category openacc flang:semantics labels Apr 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 21, 2025

@llvm/pr-subscribers-openacc

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

Changes

The async clause was not handed in a similar way on serial, parallel and kernels directive. This patches updates the ACC.td file and the flang semantic to make it homogenous.

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

5 Files Affected:

  • (modified) flang/lib/Semantics/check-acc-structure.cpp (+6-1)
  • (modified) flang/test/Semantics/OpenACC/acc-kernels.f90 (+11)
  • (modified) flang/test/Semantics/OpenACC/acc-parallel.f90 (+11)
  • (modified) flang/test/Semantics/OpenACC/acc-serial.f90 (+8-1)
  • (modified) llvm/include/llvm/Frontend/OpenACC/ACC.td (+20-24)
diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index 7afc285e7b9ae..9cbea9742a6a2 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -374,7 +374,6 @@ void AccStructureChecker::Leave(const parser::OpenACCCacheConstruct &x) {
 
 // Clause checkers
 CHECK_SIMPLE_CLAUSE(Auto, ACCC_auto)
-CHECK_SIMPLE_CLAUSE(Async, ACCC_async)
 CHECK_SIMPLE_CLAUSE(Attach, ACCC_attach)
 CHECK_SIMPLE_CLAUSE(Bind, ACCC_bind)
 CHECK_SIMPLE_CLAUSE(Capture, ACCC_capture)
@@ -444,6 +443,12 @@ void AccStructureChecker::CheckMultipleOccurrenceInDeclare(
   CheckMultipleOccurrenceInDeclare(objectList, clause);
 }
 
+void AccStructureChecker::Enter(const parser::AccClause::Async &c) {
+  llvm::acc::Clause crtClause = llvm::acc::Clause::ACCC_async;
+  CheckAllowed(crtClause);
+  CheckAllowedOncePerGroup(crtClause, llvm::acc::Clause::ACCC_device_type);
+}
+
 void AccStructureChecker::Enter(const parser::AccClause::Create &c) {
   CheckAllowed(llvm::acc::Clause::ACCC_create);
   const auto &modifierClause{c.v};
diff --git a/flang/test/Semantics/OpenACC/acc-kernels.f90 b/flang/test/Semantics/OpenACC/acc-kernels.f90
index 33e253ca8d874..44e532a9012bf 100644
--- a/flang/test/Semantics/OpenACC/acc-kernels.f90
+++ b/flang/test/Semantics/OpenACC/acc-kernels.f90
@@ -37,6 +37,17 @@ program openacc_kernels_validity
   !$acc kernels async(async1)
   !$acc end kernels
 
+  !ERROR: At most one ASYNC clause can appear on the KERNELS directive or in group separated by the DEVICE_TYPE clause
+  !$acc kernels async(async1) async(2)
+  !$acc end kernels
+
+  !$acc kernels async(async1) device_type(multicore) async(2) ! ok
+  !$acc end kernels
+
+  !ERROR: At most one ASYNC clause can appear on the KERNELS directive or in group separated by the DEVICE_TYPE clause
+  !$acc kernels async(async1) device_type(multicore) async(2) async(3)
+  !$acc end kernels
+
   !$acc kernels wait(wait1)
   !$acc end kernels
 
diff --git a/flang/test/Semantics/OpenACC/acc-parallel.f90 b/flang/test/Semantics/OpenACC/acc-parallel.f90
index 3f17d8fc862a6..b9d989ec6fced 100644
--- a/flang/test/Semantics/OpenACC/acc-parallel.f90
+++ b/flang/test/Semantics/OpenACC/acc-parallel.f90
@@ -34,6 +34,17 @@ program openacc_parallel_validity
   !$acc parallel async(1)
   !$acc end parallel
 
+  !ERROR: At most one ASYNC clause can appear on the PARALLEL directive or in group separated by the DEVICE_TYPE clause
+  !$acc parallel async(1) async(2)
+  !$acc end parallel
+
+  !$acc parallel async(1) device_type(nvidia) async(3)
+  !$acc end parallel
+
+  !ERROR: At most one ASYNC clause can appear on the PARALLEL directive or in group separated by the DEVICE_TYPE clause
+  !$acc parallel async(1) device_type(nvidia) async(2) async(3)
+  !$acc end parallel
+
   !$acc parallel async(async1)
   !$acc end parallel
 
diff --git a/flang/test/Semantics/OpenACC/acc-serial.f90 b/flang/test/Semantics/OpenACC/acc-serial.f90
index a23daecce8dd3..1f22003ed6b11 100644
--- a/flang/test/Semantics/OpenACC/acc-serial.f90
+++ b/flang/test/Semantics/OpenACC/acc-serial.f90
@@ -52,10 +52,17 @@ program openacc_serial_validity
   !$acc serial async(1)
   !$acc end serial
 
-  !ERROR: At most one ASYNC clause can appear on the SERIAL directive
+  !ERROR: At most one ASYNC clause can appear on the SERIAL directive or in group separated by the DEVICE_TYPE clause
   !$acc serial async(1) async(2)
   !$acc end serial
 
+  !ERROR: At most one ASYNC clause can appear on the SERIAL directive or in group separated by the DEVICE_TYPE clause
+  !$acc serial async(1) device_type(nvidia) async(2) async(4)
+  !$acc end serial
+
+  !$acc serial async(1) device_type(nvidia) async(2)
+  !$acc end serial
+
   !$acc serial async(async1)
   !$acc end serial
 
diff --git a/llvm/include/llvm/Frontend/OpenACC/ACC.td b/llvm/include/llvm/Frontend/OpenACC/ACC.td
index 6f6539c1ffebc..7dc97e6c89bee 100644
--- a/llvm/include/llvm/Frontend/OpenACC/ACC.td
+++ b/llvm/include/llvm/Frontend/OpenACC/ACC.td
@@ -325,7 +325,8 @@ def ACC_Declare : Directive<"declare"> {
 
 // 2.5.3
 def ACC_Kernels : Directive<"kernels"> {
-  let allowedClauses = [VersionedClause<ACCC_Attach>,
+  let allowedClauses = [VersionedClause<ACCC_Async>,
+                        VersionedClause<ACCC_Attach>,
                         VersionedClause<ACCC_Copy>,
                         VersionedClause<ACCC_Copyin>,
                         VersionedClause<ACCC_Copyout>,
@@ -338,8 +339,7 @@ def ACC_Kernels : Directive<"kernels"> {
                         VersionedClause<ACCC_DevicePtr>,
                         VersionedClause<ACCC_VectorLength>,
                         VersionedClause<ACCC_Wait>];
-  let allowedOnceClauses = [VersionedClause<ACCC_Async>,
-                            VersionedClause<ACCC_Default>,
+  let allowedOnceClauses = [VersionedClause<ACCC_Default>,
                             VersionedClause<ACCC_If>,
                             VersionedClause<ACCC_Self>];
   let association = AS_Block;
@@ -380,27 +380,23 @@ def ACC_Parallel : Directive<"parallel"> {
 def ACC_Serial : Directive<"serial"> {
   // Spec line 950-951: clause is as for the parallel construct except that the
   // num_gangs, num_workers, and vector_length clauses are not permitted.
-  let allowedClauses = [
-    VersionedClause<ACCC_Attach>,
-    VersionedClause<ACCC_Copy>,
-    VersionedClause<ACCC_Copyin>,
-    VersionedClause<ACCC_Copyout>,
-    VersionedClause<ACCC_Create>,
-    VersionedClause<ACCC_DevicePtr>,
-    VersionedClause<ACCC_DeviceType>,
-    VersionedClause<ACCC_NoCreate>,
-    VersionedClause<ACCC_Present>,
-    VersionedClause<ACCC_Private>,
-    VersionedClause<ACCC_FirstPrivate>,
-    VersionedClause<ACCC_Reduction>,
-    VersionedClause<ACCC_Wait>
-  ];
-  let allowedOnceClauses = [
-    VersionedClause<ACCC_Async>,
-    VersionedClause<ACCC_Default>,
-    VersionedClause<ACCC_If>,
-    VersionedClause<ACCC_Self>
-  ];
+  let allowedClauses = [VersionedClause<ACCC_Async>,
+                        VersionedClause<ACCC_Attach>,
+                        VersionedClause<ACCC_Copy>,
+                        VersionedClause<ACCC_Copyin>,
+                        VersionedClause<ACCC_Copyout>,
+                        VersionedClause<ACCC_Create>,
+                        VersionedClause<ACCC_DevicePtr>,
+                        VersionedClause<ACCC_DeviceType>,
+                        VersionedClause<ACCC_NoCreate>,
+                        VersionedClause<ACCC_Present>,
+                        VersionedClause<ACCC_Private>,
+                        VersionedClause<ACCC_FirstPrivate>,
+                        VersionedClause<ACCC_Reduction>,
+                        VersionedClause<ACCC_Wait>];
+  let allowedOnceClauses = [VersionedClause<ACCC_Default>,
+                            VersionedClause<ACCC_If>,
+                            VersionedClause<ACCC_Self>];
   let association = AS_Block;
   let category = CA_Executable;
 }

@llvmbot
Copy link
Member

llvmbot commented Apr 21, 2025

@llvm/pr-subscribers-flang-semantics

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

Changes

The async clause was not handed in a similar way on serial, parallel and kernels directive. This patches updates the ACC.td file and the flang semantic to make it homogenous.

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

5 Files Affected:

  • (modified) flang/lib/Semantics/check-acc-structure.cpp (+6-1)
  • (modified) flang/test/Semantics/OpenACC/acc-kernels.f90 (+11)
  • (modified) flang/test/Semantics/OpenACC/acc-parallel.f90 (+11)
  • (modified) flang/test/Semantics/OpenACC/acc-serial.f90 (+8-1)
  • (modified) llvm/include/llvm/Frontend/OpenACC/ACC.td (+20-24)
diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index 7afc285e7b9ae..9cbea9742a6a2 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -374,7 +374,6 @@ void AccStructureChecker::Leave(const parser::OpenACCCacheConstruct &x) {
 
 // Clause checkers
 CHECK_SIMPLE_CLAUSE(Auto, ACCC_auto)
-CHECK_SIMPLE_CLAUSE(Async, ACCC_async)
 CHECK_SIMPLE_CLAUSE(Attach, ACCC_attach)
 CHECK_SIMPLE_CLAUSE(Bind, ACCC_bind)
 CHECK_SIMPLE_CLAUSE(Capture, ACCC_capture)
@@ -444,6 +443,12 @@ void AccStructureChecker::CheckMultipleOccurrenceInDeclare(
   CheckMultipleOccurrenceInDeclare(objectList, clause);
 }
 
+void AccStructureChecker::Enter(const parser::AccClause::Async &c) {
+  llvm::acc::Clause crtClause = llvm::acc::Clause::ACCC_async;
+  CheckAllowed(crtClause);
+  CheckAllowedOncePerGroup(crtClause, llvm::acc::Clause::ACCC_device_type);
+}
+
 void AccStructureChecker::Enter(const parser::AccClause::Create &c) {
   CheckAllowed(llvm::acc::Clause::ACCC_create);
   const auto &modifierClause{c.v};
diff --git a/flang/test/Semantics/OpenACC/acc-kernels.f90 b/flang/test/Semantics/OpenACC/acc-kernels.f90
index 33e253ca8d874..44e532a9012bf 100644
--- a/flang/test/Semantics/OpenACC/acc-kernels.f90
+++ b/flang/test/Semantics/OpenACC/acc-kernels.f90
@@ -37,6 +37,17 @@ program openacc_kernels_validity
   !$acc kernels async(async1)
   !$acc end kernels
 
+  !ERROR: At most one ASYNC clause can appear on the KERNELS directive or in group separated by the DEVICE_TYPE clause
+  !$acc kernels async(async1) async(2)
+  !$acc end kernels
+
+  !$acc kernels async(async1) device_type(multicore) async(2) ! ok
+  !$acc end kernels
+
+  !ERROR: At most one ASYNC clause can appear on the KERNELS directive or in group separated by the DEVICE_TYPE clause
+  !$acc kernels async(async1) device_type(multicore) async(2) async(3)
+  !$acc end kernels
+
   !$acc kernels wait(wait1)
   !$acc end kernels
 
diff --git a/flang/test/Semantics/OpenACC/acc-parallel.f90 b/flang/test/Semantics/OpenACC/acc-parallel.f90
index 3f17d8fc862a6..b9d989ec6fced 100644
--- a/flang/test/Semantics/OpenACC/acc-parallel.f90
+++ b/flang/test/Semantics/OpenACC/acc-parallel.f90
@@ -34,6 +34,17 @@ program openacc_parallel_validity
   !$acc parallel async(1)
   !$acc end parallel
 
+  !ERROR: At most one ASYNC clause can appear on the PARALLEL directive or in group separated by the DEVICE_TYPE clause
+  !$acc parallel async(1) async(2)
+  !$acc end parallel
+
+  !$acc parallel async(1) device_type(nvidia) async(3)
+  !$acc end parallel
+
+  !ERROR: At most one ASYNC clause can appear on the PARALLEL directive or in group separated by the DEVICE_TYPE clause
+  !$acc parallel async(1) device_type(nvidia) async(2) async(3)
+  !$acc end parallel
+
   !$acc parallel async(async1)
   !$acc end parallel
 
diff --git a/flang/test/Semantics/OpenACC/acc-serial.f90 b/flang/test/Semantics/OpenACC/acc-serial.f90
index a23daecce8dd3..1f22003ed6b11 100644
--- a/flang/test/Semantics/OpenACC/acc-serial.f90
+++ b/flang/test/Semantics/OpenACC/acc-serial.f90
@@ -52,10 +52,17 @@ program openacc_serial_validity
   !$acc serial async(1)
   !$acc end serial
 
-  !ERROR: At most one ASYNC clause can appear on the SERIAL directive
+  !ERROR: At most one ASYNC clause can appear on the SERIAL directive or in group separated by the DEVICE_TYPE clause
   !$acc serial async(1) async(2)
   !$acc end serial
 
+  !ERROR: At most one ASYNC clause can appear on the SERIAL directive or in group separated by the DEVICE_TYPE clause
+  !$acc serial async(1) device_type(nvidia) async(2) async(4)
+  !$acc end serial
+
+  !$acc serial async(1) device_type(nvidia) async(2)
+  !$acc end serial
+
   !$acc serial async(async1)
   !$acc end serial
 
diff --git a/llvm/include/llvm/Frontend/OpenACC/ACC.td b/llvm/include/llvm/Frontend/OpenACC/ACC.td
index 6f6539c1ffebc..7dc97e6c89bee 100644
--- a/llvm/include/llvm/Frontend/OpenACC/ACC.td
+++ b/llvm/include/llvm/Frontend/OpenACC/ACC.td
@@ -325,7 +325,8 @@ def ACC_Declare : Directive<"declare"> {
 
 // 2.5.3
 def ACC_Kernels : Directive<"kernels"> {
-  let allowedClauses = [VersionedClause<ACCC_Attach>,
+  let allowedClauses = [VersionedClause<ACCC_Async>,
+                        VersionedClause<ACCC_Attach>,
                         VersionedClause<ACCC_Copy>,
                         VersionedClause<ACCC_Copyin>,
                         VersionedClause<ACCC_Copyout>,
@@ -338,8 +339,7 @@ def ACC_Kernels : Directive<"kernels"> {
                         VersionedClause<ACCC_DevicePtr>,
                         VersionedClause<ACCC_VectorLength>,
                         VersionedClause<ACCC_Wait>];
-  let allowedOnceClauses = [VersionedClause<ACCC_Async>,
-                            VersionedClause<ACCC_Default>,
+  let allowedOnceClauses = [VersionedClause<ACCC_Default>,
                             VersionedClause<ACCC_If>,
                             VersionedClause<ACCC_Self>];
   let association = AS_Block;
@@ -380,27 +380,23 @@ def ACC_Parallel : Directive<"parallel"> {
 def ACC_Serial : Directive<"serial"> {
   // Spec line 950-951: clause is as for the parallel construct except that the
   // num_gangs, num_workers, and vector_length clauses are not permitted.
-  let allowedClauses = [
-    VersionedClause<ACCC_Attach>,
-    VersionedClause<ACCC_Copy>,
-    VersionedClause<ACCC_Copyin>,
-    VersionedClause<ACCC_Copyout>,
-    VersionedClause<ACCC_Create>,
-    VersionedClause<ACCC_DevicePtr>,
-    VersionedClause<ACCC_DeviceType>,
-    VersionedClause<ACCC_NoCreate>,
-    VersionedClause<ACCC_Present>,
-    VersionedClause<ACCC_Private>,
-    VersionedClause<ACCC_FirstPrivate>,
-    VersionedClause<ACCC_Reduction>,
-    VersionedClause<ACCC_Wait>
-  ];
-  let allowedOnceClauses = [
-    VersionedClause<ACCC_Async>,
-    VersionedClause<ACCC_Default>,
-    VersionedClause<ACCC_If>,
-    VersionedClause<ACCC_Self>
-  ];
+  let allowedClauses = [VersionedClause<ACCC_Async>,
+                        VersionedClause<ACCC_Attach>,
+                        VersionedClause<ACCC_Copy>,
+                        VersionedClause<ACCC_Copyin>,
+                        VersionedClause<ACCC_Copyout>,
+                        VersionedClause<ACCC_Create>,
+                        VersionedClause<ACCC_DevicePtr>,
+                        VersionedClause<ACCC_DeviceType>,
+                        VersionedClause<ACCC_NoCreate>,
+                        VersionedClause<ACCC_Present>,
+                        VersionedClause<ACCC_Private>,
+                        VersionedClause<ACCC_FirstPrivate>,
+                        VersionedClause<ACCC_Reduction>,
+                        VersionedClause<ACCC_Wait>];
+  let allowedOnceClauses = [VersionedClause<ACCC_Default>,
+                            VersionedClause<ACCC_If>,
+                            VersionedClause<ACCC_Self>];
   let association = AS_Block;
   let category = CA_Executable;
 }

@clementval clementval merged commit 3ceb3d9 into llvm:main Apr 21, 2025
15 checks passed
@clementval clementval deleted the acc_async branch April 21, 2025 21:48
erichkeane added a commit to erichkeane/llvm-project that referenced this pull request Apr 23, 2025
@erichkeane
[Flang][OpenACC] Make async clause on data consistent with elsewhere
99288c9 
in llvm#136610 we agreed that all async clauses on compute constructs should act
as 'only 1 per device-type-group'.  On `data`, it has the same
specification language, and the same real requirements, so it seems
sensible to make it work the same way.
@erichkeane erichkeane mentioned this pull request Apr 23, 2025
Merged
erichkeane added a commit that referenced this pull request Apr 23, 2025
@erichkeane
[Flang][OpenACC] Make async clause on data consistent with elsewhere (#…
2e389cb 
…136866)

in #136610 we agreed that all async clauses on compute constructs should
act as 'only 1 per device-type-group'. On `data`, it has the same
specification language, and the same real requirements, so it seems
sensible to make it work the same way.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
@clementval @IanWood1
[flang][openacc] Make async clause behavior homogenous (llvm#136610)
a813dd3 
The `async` clause was not handed in a similar way on `serial`,
`parallel` and `kernels` directive. This patches updates the `ACC.td`
file and the flang semantic to make it homogenous.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
@erichkeane @IanWood1
[Flang][OpenACC] Make async clause on data consistent with elsewhere (l…
a806300 
…lvm#136866)

in llvm#136610 we agreed that all async clauses on compute constructs should
act as 'only 1 per device-type-group'. On `data`, it has the same
specification language, and the same real requirements, so it seems
sensible to make it work the same way.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
@clementval @IanWood1
[flang][openacc] Make async clause behavior homogenous (llvm#136610)
ef2299d 
The `async` clause was not handed in a similar way on `serial`,
`parallel` and `kernels` directive. This patches updates the `ACC.td`
file and the flang semantic to make it homogenous.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
@erichkeane @IanWood1
[Flang][OpenACC] Make async clause on data consistent with elsewhere (l…
9b48960 
…lvm#136866)

in llvm#136610 we agreed that all async clauses on compute constructs should
act as 'only 1 per device-type-group'. On `data`, it has the same
specification language, and the same real requirements, so it seems
sensible to make it work the same way.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
@clementval @IanWood1
[flang][openacc] Make async clause behavior homogenous (llvm#136610)
c481bb7 
The `async` clause was not handed in a similar way on `serial`,
`parallel` and `kernels` directive. This patches updates the `ACC.td`
file and the flang semantic to make it homogenous.
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
@erichkeane @IanWood1
[Flang][OpenACC] Make async clause on data consistent with elsewhere (l…
b790fc5 
…lvm#136866)

in llvm#136610 we agreed that all async clauses on compute constructs should
act as 'only 1 per device-type-group'. On `data`, it has the same
specification language, and the same real requirements, so it seems
sensible to make it work the same way.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erichkeane erichkeane erichkeane approved these changes

@razvanlupusoru razvanlupusoru Awaiting requested review from razvanlupusoru

Assignees
No one assigned
Labels
flang:semantics flang Flang issues not falling into any other category openacc
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@clementval @llvmbot @erichkeane