[flang][OpenMP]: Allow orphaned distribute construct

[chandraghale]

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.

[llvmbot]

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-semantics

Author: CHANDRA GHALE (chandraghale)

Changes

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.

---

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

5 Files Affected:

• (modified) flang/lib/Semantics/check-omp-loop.cpp (+9-10)
• (modified) flang/test/Semantics/OpenMP/combined-constructs.f90 (+1-1)
• (modified) flang/test/Semantics/OpenMP/do05.f90 (+2-2)
• (modified) flang/test/Semantics/OpenMP/linear-iter.f90 (+3-3)
• (modified) flang/test/Semantics/OpenMP/nested-distribute.f90 (+13)

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 

[tblah]

Thanks! Just one question.

[Stylie777]

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.

[chandraghale]

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
Yes, there are cases where the original error message is emitted. For example lexical violations (e.g., distribute directly under, or after, a non-teams context) still produce the original error, in code like:

!$omp end parallel !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region. !$omp distribute firstprivate(a) do i = 1, 10 j = j + 1 end do 

Original error message is still relevant so kept it.

[Stylie777]

Thanks @chandraghale. LGTM!

[tblah]

LGTM