-
Notifications
You must be signed in to change notification settings - Fork 15.1k
[flang][OpenMP]: Allow orphaned distribute construct #163546
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
Conversation
|
@llvm/pr-subscribers-flang-openmp @llvm/pr-subscribers-flang-semantics Author: CHANDRA GHALE (chandraghale) ChangesIf there is a call inside a TEAMS construct, and that call contains a DISTRIBUTE construct, the DISTRIBUTE region is considered to be enclosed by the TEAMS region (based on the dynamic extent of the construct). Currently, Flang diagnoses this as an error, which is incorrect. This patch adjusts the nesting check for the OpenMP DISTRIBUTE directive. It retains the error for DISTRIBUTE directives that are incorrectly nested lexically but downgrades it to a warning for orphaned directives to allow dynamic nesting, such as when a subroutine with DISTRIBUTE is called from within a TEAMS region. Full diff: https://github.com/llvm/llvm-project/pull/163546.diff 5 Files Affected:
diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp
index c9d0495850b6e..331397b035cf9 100644
--- a/flang/lib/Semantics/check-omp-loop.cpp
+++ b/flang/lib/Semantics/check-omp-loop.cpp
@@ -127,24 +127,23 @@ using namespace Fortran::semantics::omp;
void OmpStructureChecker::HasInvalidDistributeNesting(
const parser::OpenMPLoopConstruct &x) {
- bool violation{false};
const parser::OmpDirectiveName &beginName{x.BeginDir().DirName()};
if (llvm::omp::topDistributeSet.test(beginName.v)) {
// `distribute` region has to be nested
- if (!CurrentDirectiveIsNested()) {
- violation = true;
- } else {
+ if (CurrentDirectiveIsNested()) {
// `distribute` region has to be strictly nested inside `teams`
if (!llvm::omp::bottomTeamsSet.test(GetContextParent().directive)) {
- violation = true;
+ context_.Say(beginName.source,
+ "`DISTRIBUTE` region has to be strictly nested inside `TEAMS` "
+ "region."_err_en_US);
}
+ } else {
+ // If not lexically nested (orphaned), issue a warning.
+ context_.Say(beginName.source,
+ "`DISTRIBUTE` must be dynamically enclosed in a `TEAMS` "
+ "region."_warn_en_US);
}
}
- if (violation) {
- context_.Say(beginName.source,
- "`DISTRIBUTE` region has to be strictly nested inside `TEAMS` "
- "region."_err_en_US);
- }
}
void OmpStructureChecker::HasInvalidLoopBinding(
const parser::OpenMPLoopConstruct &x) {
diff --git a/flang/test/Semantics/OpenMP/combined-constructs.f90 b/flang/test/Semantics/OpenMP/combined-constructs.f90
index 49da56298d426..3e9c65434f695 100644
--- a/flang/test/Semantics/OpenMP/combined-constructs.f90
+++ b/flang/test/Semantics/OpenMP/combined-constructs.f90
@@ -7,7 +7,7 @@ program main
real(8) :: a(256), b(256)
N = 256
- !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+ !WARNING: `DISTRIBUTE` must be dynamically enclosed in a `TEAMS` region.
!$omp distribute simd
do i = 1, N
a(i) = 3.14d0
diff --git a/flang/test/Semantics/OpenMP/do05.f90 b/flang/test/Semantics/OpenMP/do05.f90
index 24844f9fe4f62..d8320520879ef 100644
--- a/flang/test/Semantics/OpenMP/do05.f90
+++ b/flang/test/Semantics/OpenMP/do05.f90
@@ -49,7 +49,7 @@ program omp_do
end do
!$omp end parallel do simd
- !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+ !WARNING: `DISTRIBUTE` must be dynamically enclosed in a `TEAMS` region.
!$omp distribute parallel do
do i=1,10
if( i == 3 ) then
@@ -64,7 +64,7 @@ program omp_do
end do
!$omp end distribute parallel do
- !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+ !WARNING: `DISTRIBUTE` must be dynamically enclosed in a `TEAMS` region.
!$omp distribute parallel do simd
do i=1,10
if( i == 3 ) then
diff --git a/flang/test/Semantics/OpenMP/linear-iter.f90 b/flang/test/Semantics/OpenMP/linear-iter.f90
index 1f40228be92ad..5c9f2d5fb34c4 100644
--- a/flang/test/Semantics/OpenMP/linear-iter.f90
+++ b/flang/test/Semantics/OpenMP/linear-iter.f90
@@ -53,7 +53,7 @@ SUBROUTINE LINEAR_BAD(N)
!$omp end teams
!$omp end target
- !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+ !WARNING: `DISTRIBUTE` must be dynamically enclosed in a `TEAMS` region.
!ERROR: Variable 'j' not allowed in LINEAR clause, only loop iterator can be specified in LINEAR clause of a construct combined with DISTRIBUTE
!$omp distribute simd linear(i,j)
do i = 1, N
@@ -63,7 +63,7 @@ SUBROUTINE LINEAR_BAD(N)
enddo
!$omp end distribute simd
- !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+ !WARNING: `DISTRIBUTE` must be dynamically enclosed in a `TEAMS` region.
!ERROR: Variable 'j' not allowed in LINEAR clause, only loop iterator can be specified in LINEAR clause of a construct combined with DISTRIBUTE
!$omp distribute simd linear(i,j) collapse(1)
do i = 1, N
@@ -73,7 +73,7 @@ SUBROUTINE LINEAR_BAD(N)
enddo
!$omp end distribute simd
- !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region.
+ !WARNING: `DISTRIBUTE` must be dynamically enclosed in a `TEAMS` region.
!$omp distribute simd linear(i,j) collapse(2)
do i = 1, N
do j = 1, N
diff --git a/flang/test/Semantics/OpenMP/nested-distribute.f90 b/flang/test/Semantics/OpenMP/nested-distribute.f90
index cb4aea3bcdffa..4134ced7d81f6 100644
--- a/flang/test/Semantics/OpenMP/nested-distribute.f90
+++ b/flang/test/Semantics/OpenMP/nested-distribute.f90
@@ -1,6 +1,15 @@
! RUN: %python %S/../test_errors.py %s %flang -fopenmp
! Check OpenMP clause validity for the following directives:
! 2.10 Device constructs
+
+subroutine f
+ integer :: i
+ !WARNING: `DISTRIBUTE` must be dynamically enclosed in a `TEAMS` region.
+ !$omp distribute
+ do i = 1, 100
+ print *, "hello"
+ end do
+end subroutine
program main
real(8) :: arrayA(256), arrayB(256)
@@ -108,4 +117,8 @@ program main
end do
!$omp end distribute
!$omp end task
+
+ !$omp teams
+ call foo
+ !$omp end teams
end program main
|
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.
Thanks! Just one question.
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.
I am concerned about the changes to some of the tests in this PR. It looks like errors, that should still be genuine errors, are being changed to warnings. We are not testing the error at all so at the very least, tests for that need adding.
EDIT: Sorry, I have resolved my threads as I originally misunderstood some aims of this PR.
Is there a case we will now hit the original error message? If so we should still test this but if not, is there a case for removing it and only having the new warning in the code.
Thanks @Stylie777 Original error message is still relevant so kept it. |
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.
Thanks @chandraghale. LGTM!
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.
LGTM
If there is a call inside a TEAMS construct, and that call contains a
DISTRIBUTE construct, the DISTRIBUTE region is considered to be enclosed
by the TEAMS region (based on the dynamic extent of the construct).
Currently, Flang diagnoses this as an error, which is incorrect.
For eg :
```
subroutine f
!$omp distribute
do i = 1, 100
...
end do
end subroutine
subroutine g
!$omp teams
call f ! this call is ok, distribute enclosed by teams
!$omp end teams
end subroutine
```
This patch adjusts the nesting check for the OpenMP DISTRIBUTE
directive. It retains the error for DISTRIBUTE directives that are
incorrectly nested lexically but downgrades it to a warning for orphaned
directives to allow dynamic nesting, such as when a subroutine with
DISTRIBUTE is called from within a TEAMS region.
Co-authored-by: Chandra Ghale <ghale@pe31.hpc.amslabs.hpecorp.net>
If there is a call inside a TEAMS construct, and that call contains a
DISTRIBUTE construct, the DISTRIBUTE region is considered to be enclosed
by the TEAMS region (based on the dynamic extent of the construct).
Currently, Flang diagnoses this as an error, which is incorrect.
For eg :
```
subroutine f
!$omp distribute
do i = 1, 100
...
end do
end subroutine
subroutine g
!$omp teams
call f ! this call is ok, distribute enclosed by teams
!$omp end teams
end subroutine
```
This patch adjusts the nesting check for the OpenMP DISTRIBUTE
directive. It retains the error for DISTRIBUTE directives that are
incorrectly nested lexically but downgrades it to a warning for orphaned
directives to allow dynamic nesting, such as when a subroutine with
DISTRIBUTE is called from within a TEAMS region.
Co-authored-by: Chandra Ghale <ghale@pe31.hpc.amslabs.hpecorp.net>
If there is a call inside a TEAMS construct, and that call contains a DISTRIBUTE construct, the DISTRIBUTE region is considered to be enclosed by the TEAMS region (based on the dynamic extent of the construct). Currently, Flang diagnoses this as an error, which is incorrect.
For eg :
This patch adjusts the nesting check for the OpenMP DISTRIBUTE directive. It retains the error for DISTRIBUTE directives that are incorrectly nested lexically but downgrades it to a warning for orphaned directives to allow dynamic nesting, such as when a subroutine with DISTRIBUTE is called from within a TEAMS region.