[Flang][OpenMP] Add semantic checks for order clause #115281
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Thirumalai-Shaktivel
requested review from
NimishMishra,
kaviya2510,
kiranchandramohan,
kiranktp,
kparzysz and
tblah
llvmbot
added
flang
Member
|
@llvm/pr-subscribers-flang-openmp Author: Thirumalai Shaktivel (Thirumalai-Shaktivel) ChangesFix:
OpenMP 5.2:
OpenMP 5.1:
Full diff: https://github.com/llvm/llvm-project/pull/115281.diff 4 Files Affected:
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 014604627f2cd1..cb87ec860d7028 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -598,6 +598,46 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
CheckDistLinear(x);
}
}
+
+// OpenMP 5.2: 10.3 Order clause restrictions
+void OmpStructureChecker::Enter(const parser::ProcedureDesignator &x) {
+ if (!dirContext_.empty() &&
+ (llvm::omp::allDoSet | llvm::omp::allSimdSet |
+ llvm::omp::allDistributeSet)
+ .test(GetContext().directive)) {
+ if (FindClause(llvm::omp::Clause::OMPC_order)) {
+ const auto &name{std::get<parser::Name>(x.u)};
+ if (std::get<parser::OmpOrderClause::Type>(orderClause.v.t) ==
+ parser::OmpOrderClause::Type::Concurrent &&
+ llvm::StringRef(name.ToString()).starts_with_insensitive("omp_")) {
+ context_.Say(name.source,
+ "The OpenMP runtime API calls are not allowed in "
+ "the `order(concurrent)` clause region"_err_en_US);
+ }
+ }
+ }
+}
+
+// OpenMP 5.2: 10.3 Order clause restrictions
+void OmpStructureChecker::Enter(const parser::Designator &x) {
+ if (!dirContext_.empty() &&
+ (llvm::omp::allDoSet | llvm::omp::allSimdSet |
+ llvm::omp::allDistributeSet)
+ .test(GetContext().directive)) {
+ if (const auto *clause{FindClause(llvm::omp::Clause::OMPC_order)}) {
+ const auto &orderClause{std::get<parser::OmpClause::Order>(clause->u)};
+ const auto &name{parser::Unwrap<parser::Name>(x.u)};
+ if (std::get<parser::OmpOrderClause::Type>(orderClause.v.t) ==
+ parser::OmpOrderClause::Type::Concurrent &&
+ name->symbol->test(Symbol::Flag::OmpThreadprivate)) {
+ context_.Say(name->source,
+ "A THREADPRIVATE variable cannot appear in an `order(concurrent)` "
+ "clause region, the behavior is unspecified"_err_en_US);
+ }
+ }
+ }
+}
+
const parser::Name OmpStructureChecker::GetLoopIndex(
const parser::DoConstruct *x) {
using Bounds = parser::LoopControl::Bounds;
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index d9236be8bced4f..690805f9a9d66b 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -131,6 +131,9 @@ class OmpStructureChecker
void Enter(const parser::OmpAtomicCapture &);
void Leave(const parser::OmpAtomic &);
+ void Enter(const parser::ProcedureDesignator &);
+ void Enter(const parser::Designator &);
+
#define GEN_FLANG_CLAUSE_CHECK_ENTER
#include "llvm/Frontend/OpenMP/OMP.inc"
diff --git a/flang/test/Semantics/OpenMP/order-clause02.f90 b/flang/test/Semantics/OpenMP/order-clause02.f90
new file mode 100644
index 00000000000000..c39fb27d657fcc
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/order-clause02.f90
@@ -0,0 +1,64 @@
+! REQUIRES: openmp_runtime
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=50
+! OpenMP Version 5.2
+! Various checks for the order clause
+! 10.3 `order` Clause
+
+! Case 1
+subroutine omp_order_runtime_api_call_01()
+ use omp_lib
+ integer :: i
+ !$omp do order(concurrent)
+ do i = 1, 5
+ !ERROR: The OpenMP runtime API calls are not allowed in the `order(concurrent)` clause region
+ print*, omp_get_thread_num()
+ end do
+ !$omp end do
+end subroutine omp_order_runtime_api_call_01
+
+subroutine omp_order_runtime_api_call_02()
+ use omp_lib
+ integer :: i, num_threads
+ !$omp do order(concurrent)
+ do i = 1, 5
+ !ERROR: The OpenMP runtime API calls are not allowed in the `order(concurrent)` clause region
+ call omp_set_num_threads(num_threads)
+ end do
+ !$omp end do
+end subroutine omp_order_runtime_api_call_02
+
+! Case 2
+subroutine test_order_threadprivate()
+ integer :: i, j = 1, x
+ !$omp threadprivate(j)
+ !$omp parallel do order(concurrent)
+ do i = 1, 5
+ !ERROR: A THREADPRIVATE variable cannot appear in an `order(concurrent)` clause region, the behavior is unspecified
+ j = x + 1
+ end do
+ !$omp end parallel do
+end subroutine
+
+! Case 3
+subroutine omp_order_duplicate_01()
+ implicit none
+ integer :: i, j
+ !ERROR: At most one ORDER clause can appear on the TARGET PARALLEL DO SIMD directive
+ !$OMP target parallel do simd ORDER(concurrent) ORDER(concurrent)
+ do i = 1, 5
+ j = j + 1
+ end do
+ !$omp end target parallel do simd
+end subroutine
+
+subroutine omp_order_duplicate_02()
+ integer :: i, j
+ !$omp teams
+ !ERROR: At most one ORDER clause can appear on the DISTRIBUTE PARALLEL DO SIMD directive
+ !$omp distribute parallel do simd order(concurrent) order(concurrent)
+ do i = 1, 5
+ j = j + 1
+ end do
+ !$omp end distribute parallel do simd
+ !$omp end teams
+end subroutine
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index 36834939d9b451..29f6e65f43b38a 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -1235,7 +1235,6 @@ def OMP_DistributeParallelDoSimd : Directive<"distribute parallel do simd"> {
VersionedClause<OMPC_Linear>,
VersionedClause<OMPC_NonTemporal>,
VersionedClause<OMPC_NumThreads>,
- VersionedClause<OMPC_Order, 50>,
VersionedClause<OMPC_Private>,
VersionedClause<OMPC_ProcBind>,
VersionedClause<OMPC_Reduction>,
@@ -1244,6 +1243,9 @@ def OMP_DistributeParallelDoSimd : Directive<"distribute parallel do simd"> {
VersionedClause<OMPC_Shared>,
VersionedClause<OMPC_SimdLen>,
];
+ let allowedOnceClauses = [
+ VersionedClause<OMPC_Order, 50>,
+ ];
let leafConstructs = [OMP_Distribute, OMP_Parallel, OMP_Do, OMP_Simd];
let category = CA_Executable;
}
@@ -1908,7 +1910,6 @@ def OMP_TargetParallelDoSimd : Directive<"target parallel do simd"> {
VersionedClause<OMPC_NonTemporal>,
VersionedClause<OMPC_NoWait>,
VersionedClause<OMPC_NumThreads>,
- VersionedClause<OMPC_Order, 50>,
VersionedClause<OMPC_Ordered>,
VersionedClause<OMPC_Private>,
VersionedClause<OMPC_ProcBind>,
@@ -1919,6 +1920,9 @@ def OMP_TargetParallelDoSimd : Directive<"target parallel do simd"> {
VersionedClause<OMPC_SimdLen>,
VersionedClause<OMPC_UsesAllocators>,
];
+ let allowedOnceClauses = [
+ VersionedClause<OMPC_Order, 50>
+ ];
let leafConstructs = [OMP_Target, OMP_Parallel, OMP_Do, OMP_Simd];
let category = CA_Executable;
}
|
Member
|
@llvm/pr-subscribers-flang-semantics Author: Thirumalai Shaktivel (Thirumalai-Shaktivel) ChangesFix:
OpenMP 5.2:
OpenMP 5.1:
Full diff: https://github.com/llvm/llvm-project/pull/115281.diff 4 Files Affected:
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 014604627f2cd1..cb87ec860d7028 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -598,6 +598,46 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
CheckDistLinear(x);
}
}
+
+// OpenMP 5.2: 10.3 Order clause restrictions
+void OmpStructureChecker::Enter(const parser::ProcedureDesignator &x) {
+ if (!dirContext_.empty() &&
+ (llvm::omp::allDoSet | llvm::omp::allSimdSet |
+ llvm::omp::allDistributeSet)
+ .test(GetContext().directive)) {
+ if (FindClause(llvm::omp::Clause::OMPC_order)) {
+ const auto &name{std::get<parser::Name>(x.u)};
+ if (std::get<parser::OmpOrderClause::Type>(orderClause.v.t) ==
+ parser::OmpOrderClause::Type::Concurrent &&
+ llvm::StringRef(name.ToString()).starts_with_insensitive("omp_")) {
+ context_.Say(name.source,
+ "The OpenMP runtime API calls are not allowed in "
+ "the `order(concurrent)` clause region"_err_en_US);
+ }
+ }
+ }
+}
+
+// OpenMP 5.2: 10.3 Order clause restrictions
+void OmpStructureChecker::Enter(const parser::Designator &x) {
+ if (!dirContext_.empty() &&
+ (llvm::omp::allDoSet | llvm::omp::allSimdSet |
+ llvm::omp::allDistributeSet)
+ .test(GetContext().directive)) {
+ if (const auto *clause{FindClause(llvm::omp::Clause::OMPC_order)}) {
+ const auto &orderClause{std::get<parser::OmpClause::Order>(clause->u)};
+ const auto &name{parser::Unwrap<parser::Name>(x.u)};
+ if (std::get<parser::OmpOrderClause::Type>(orderClause.v.t) ==
+ parser::OmpOrderClause::Type::Concurrent &&
+ name->symbol->test(Symbol::Flag::OmpThreadprivate)) {
+ context_.Say(name->source,
+ "A THREADPRIVATE variable cannot appear in an `order(concurrent)` "
+ "clause region, the behavior is unspecified"_err_en_US);
+ }
+ }
+ }
+}
+
const parser::Name OmpStructureChecker::GetLoopIndex(
const parser::DoConstruct *x) {
using Bounds = parser::LoopControl::Bounds;
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index d9236be8bced4f..690805f9a9d66b 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -131,6 +131,9 @@ class OmpStructureChecker
void Enter(const parser::OmpAtomicCapture &);
void Leave(const parser::OmpAtomic &);
+ void Enter(const parser::ProcedureDesignator &);
+ void Enter(const parser::Designator &);
+
#define GEN_FLANG_CLAUSE_CHECK_ENTER
#include "llvm/Frontend/OpenMP/OMP.inc"
diff --git a/flang/test/Semantics/OpenMP/order-clause02.f90 b/flang/test/Semantics/OpenMP/order-clause02.f90
new file mode 100644
index 00000000000000..c39fb27d657fcc
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/order-clause02.f90
@@ -0,0 +1,64 @@
+! REQUIRES: openmp_runtime
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=50
+! OpenMP Version 5.2
+! Various checks for the order clause
+! 10.3 `order` Clause
+
+! Case 1
+subroutine omp_order_runtime_api_call_01()
+ use omp_lib
+ integer :: i
+ !$omp do order(concurrent)
+ do i = 1, 5
+ !ERROR: The OpenMP runtime API calls are not allowed in the `order(concurrent)` clause region
+ print*, omp_get_thread_num()
+ end do
+ !$omp end do
+end subroutine omp_order_runtime_api_call_01
+
+subroutine omp_order_runtime_api_call_02()
+ use omp_lib
+ integer :: i, num_threads
+ !$omp do order(concurrent)
+ do i = 1, 5
+ !ERROR: The OpenMP runtime API calls are not allowed in the `order(concurrent)` clause region
+ call omp_set_num_threads(num_threads)
+ end do
+ !$omp end do
+end subroutine omp_order_runtime_api_call_02
+
+! Case 2
+subroutine test_order_threadprivate()
+ integer :: i, j = 1, x
+ !$omp threadprivate(j)
+ !$omp parallel do order(concurrent)
+ do i = 1, 5
+ !ERROR: A THREADPRIVATE variable cannot appear in an `order(concurrent)` clause region, the behavior is unspecified
+ j = x + 1
+ end do
+ !$omp end parallel do
+end subroutine
+
+! Case 3
+subroutine omp_order_duplicate_01()
+ implicit none
+ integer :: i, j
+ !ERROR: At most one ORDER clause can appear on the TARGET PARALLEL DO SIMD directive
+ !$OMP target parallel do simd ORDER(concurrent) ORDER(concurrent)
+ do i = 1, 5
+ j = j + 1
+ end do
+ !$omp end target parallel do simd
+end subroutine
+
+subroutine omp_order_duplicate_02()
+ integer :: i, j
+ !$omp teams
+ !ERROR: At most one ORDER clause can appear on the DISTRIBUTE PARALLEL DO SIMD directive
+ !$omp distribute parallel do simd order(concurrent) order(concurrent)
+ do i = 1, 5
+ j = j + 1
+ end do
+ !$omp end distribute parallel do simd
+ !$omp end teams
+end subroutine
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td
index 36834939d9b451..29f6e65f43b38a 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -1235,7 +1235,6 @@ def OMP_DistributeParallelDoSimd : Directive<"distribute parallel do simd"> {
VersionedClause<OMPC_Linear>,
VersionedClause<OMPC_NonTemporal>,
VersionedClause<OMPC_NumThreads>,
- VersionedClause<OMPC_Order, 50>,
VersionedClause<OMPC_Private>,
VersionedClause<OMPC_ProcBind>,
VersionedClause<OMPC_Reduction>,
@@ -1244,6 +1243,9 @@ def OMP_DistributeParallelDoSimd : Directive<"distribute parallel do simd"> {
VersionedClause<OMPC_Shared>,
VersionedClause<OMPC_SimdLen>,
];
+ let allowedOnceClauses = [
+ VersionedClause<OMPC_Order, 50>,
+ ];
let leafConstructs = [OMP_Distribute, OMP_Parallel, OMP_Do, OMP_Simd];
let category = CA_Executable;
}
@@ -1908,7 +1910,6 @@ def OMP_TargetParallelDoSimd : Directive<"target parallel do simd"> {
VersionedClause<OMPC_NonTemporal>,
VersionedClause<OMPC_NoWait>,
VersionedClause<OMPC_NumThreads>,
- VersionedClause<OMPC_Order, 50>,
VersionedClause<OMPC_Ordered>,
VersionedClause<OMPC_Private>,
VersionedClause<OMPC_ProcBind>,
@@ -1919,6 +1920,9 @@ def OMP_TargetParallelDoSimd : Directive<"target parallel do simd"> {
VersionedClause<OMPC_SimdLen>,
VersionedClause<OMPC_UsesAllocators>,
];
+ let allowedOnceClauses = [
+ VersionedClause<OMPC_Order, 50>
+ ];
let leafConstructs = [OMP_Target, OMP_Parallel, OMP_Do, OMP_Simd];
let category = CA_Executable;
}
|
Thirumalai-Shaktivel
force-pushed
the
llvm/order_clause_01
branch
from
5791620 to
d184299
Compare
Fix: - Move the order clause to allowedOnceClauses list in the `OMP_DistributeParallelDoSimd` and `OMP_TargetParallelDoSimd` definitions OpenMP 5.2: 10.3 Order clause restrictions - A region that corresponds to a construct with an order clause that specifies concurrent may not contain calls to procedures that contain OpenMP directives. - A region that corresponds to a construct with an order clause that specifies concurrent may not contain OpenMP runtime API calls. OpenMP 5.1: 2.11.3 order Clause restriction: - At most one order clause may appear on a construct.
Contributor
|
Regions in OpenMP are generally dynamic (and not static). Will the checks work across function/subroutine calls? |
Thirumalai-Shaktivel
force-pushed
the
llvm/order_clause_01
branch
from
d184299 to
59ff7bb
Compare
Member
Author
|
Yes, regarding that, I was wondering, how to get the details of the region? If we get the answer for this, your example would have the similar fix. |
Contributor
Generally, we do not support these checks. |
Member
Author
|
Okay, I got it. Shall we keep the following restrictions, then? |
Member
Author
|
It says "If a threadprivate variable is referenced inside a region ...". This might be not possible as well. |
Member
Author
|
Thanks for the review, @kiranchandramohan |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix:
OMP_DistributeParallelDoSimdandOMP_TargetParallelDoSimddefinitionsOpenMP 5.2:
10.3 Order clause restrictions
OpenMP 5.1:
2.11.3 order Clause restriction: