-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[Flang] - Enhance testing for strictly-nested teams in target regions. #168437
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
base: main
Are you sure you want to change the base?
Conversation
This patch enhances the semantics test for checking that teams directives are strictly nested inside target directives.
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-flang-semantics Author: Pranav Bhandarkar (bhandarkar-pranav) ChangesThis patch enhances the semantics test for checking that teams directives are strictly nested inside target directives. Fixes #153173 Full diff: https://github.com/llvm/llvm-project/pull/168437.diff 3 Files Affected:
diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp
index aaaa2d6e78280..09005e3076d03 100644
--- a/flang/lib/Semantics/check-omp-loop.cpp
+++ b/flang/lib/Semantics/check-omp-loop.cpp
@@ -262,6 +262,15 @@ void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) {
EnterDirectiveNest(SIMDNest);
}
+ if (CurrentDirectiveIsNested() &&
+ llvm::omp::allTeamsSet.test(GetContext().directive) &&
+ GetContextParent().directive == llvm::omp::Directive::OMPD_target &&
+ !GetDirectiveNest(TargetBlockOnlyTeams)) {
+ context_.Say(GetContextParent().directiveSource,
+ "TARGET construct with nested TEAMS region contains statements or "
+ "directives outside of the TEAMS construct"_err_en_US);
+ }
+
// Combined target loop constructs are target device constructs. Keep track of
// whether any such construct has been visited to later check that REQUIRES
// directives for target-related options don't appear after them.
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 37b4404cc598f..472208949abad 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -5215,6 +5215,13 @@ bool OmpStructureChecker::CheckTargetBlockOnlyTeams(
if (dirId == llvm::omp::Directive::OMPD_teams) {
nestedTeams = true;
}
+ } else if (const auto *ompLoopConstruct{
+ std::get_if<parser::OpenMPLoopConstruct>(
+ &ompConstruct->u)}) {
+ llvm::omp::Directive dirId{ompLoopConstruct->BeginDir().DirId()};
+ if (llvm::omp::allTeamsSet.test(dirId)) {
+ nestedTeams = true;
+ }
}
}
diff --git a/flang/test/Semantics/OpenMP/target-teams-nesting.f90 b/flang/test/Semantics/OpenMP/target-teams-nesting.f90
new file mode 100644
index 0000000000000..59786d4f9e397
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/target-teams-nesting.f90
@@ -0,0 +1,22 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
+
+program main
+ implicit none
+ integer, parameter :: n = 100
+ integer, parameter :: expected = n+2
+ integer :: i
+ integer :: counter
+
+ counter = 0
+ !ERROR: TARGET construct with nested TEAMS region contains statements or directives outside of the TEAMS construct
+ !$omp target map(tofrom:counter)
+ counter = counter+1
+ !$omp teams distribute reduction(+:counter)
+ do i=1, n
+ counter = counter+1
+ end do
+ counter = counter+1
+ !$omp end target
+
+ print '("Result: "I0)', counter
+ end program
|
🐧 Linux x64 Test Results
|
| counter = counter+1 | ||
| !$omp end target | ||
|
|
||
| print '("Result: "I0)', counter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: no need for the print statement here.
| if (CurrentDirectiveIsNested() && | ||
| llvm::omp::topTeamsSet.test(GetContext().directive) && | ||
| GetContextParent().directive == llvm::omp::Directive::OMPD_target && | ||
| !GetDirectiveNest(TargetBlockOnlyTeams)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not exactly relevant to this PR since a similar check is used somewhere else. But for the following example:
!$omp target ....
!$omp teams ....
!$omp target ....
.... some statement ....
!$omp teams ....
....
....
Would the semantic check trigger in this case? I think it won't since GetDirectiveNest(TargetBlockOnlyTeams) will return a non-zero result.
This patch enhances the semantics test for checking that teams directives are strictly nested inside target directives.
Fixes #153173