[flang][OpenMP][regression] Flang compiles bad openmp semantics.

[scamp-nvidia]

In our testing system, we identifed another issue that appears to be related to #168884 change of OpenMP behavior - tagging @kparzysz here again.

In the following test, we expect the compilation to fail with an error message complaining about the OMP ENDDO.

      SUBROUTINE WORK_SAN(IVAR, JVAR)
      INTEGER IVAR,JVAR
      END SUBROUTINE WORK_SAN

      SUBROUTINE DO_OMP_ISSUE
        INTEGER IVAR, JVAR

        DO 100 IVAR = 1,10
!$OMP     DO
          DO 100 JVAR = 1,10
            CALL WORK_SAN(IVAR,JVAR)
100     CONTINUE
!$OMP   ENDDO
      END SUBROUTINE DO_OMP_ISSUE

However, after that recent change, this no longer fails - it actually compiles. It fails as expected with nvfortran and gfortran.

[scamp]$ nvfortran test.F90 -c -fopenmp
NVFORTRAN-S-0155-ENDDO must immediately follow a DO loop  (test.F90: 13)
  0 inform,   0 warnings,   1 severes, 0 fatal for do_omp_issue
[scamp]$ gfortran test.F90 -c -fopenmp
test.F90:10:28:

   10 |           DO 100 JVAR = 1,10
      |                            1
Warning: Fortran 2018 deleted feature: Shared DO termination label 100 at (1)
test.F90:13:13:

   13 | !$OMP   ENDDO
      |             1
Error: Unexpected !$OMP END DO statement at (1)
[scamp]$ flang test.F90 -c -fopenmp
[scamp]$

Prior to this change, we would get a warning in line with:

./test.F90:13:9: error: The ENDDO directive must follow the DO loop associated with the loop construct
  !$OMP   ENDDO

[kparzysz]

Yes, we parse this as

SUBROUTINE do_omp_issue
 INTEGER ivar, jvar
 DO ivar=1_4,10_4
!$OMP DO
  DO jvar=1_4,10_4
    CALL work_san(ivar,jvar)
   100 CONTINUE
  END DO
!$OMP END DO
 END DO
END SUBROUTINE do_omp_issue

The "END DO" directive is allowed in OpenMP 5.1 onwards. I guess we could complain about it when -fopenmp-version is <= 5.0.

[tarunprabhu]

Is this similar to the issue with one of the gfortran tests that was recently disabled? I don't believe that there is an OpenMP version specified in the run line for that test. If they are related, a fix for this might impact that as well.

[scamp-nvidia]

Okay - if that's allowed now, then I can update our test. Though I thinking erroring for -fopenmp-version less than or equal to 5.0 would make sense.

[llvmbot]

@llvm/issue-subscribers-flang-frontend

Author: None (scamp-nvidia)

In our testing system, we identifed another issue that appears to be related to #168884 change of OpenMP behavior - tagging @kparzysz here again.

In the following test, we expect the compilation to fail with an error message complaining about the OMP ENDDO.

      SUBROUTINE WORK_SAN(IVAR, JVAR)
      INTEGER IVAR,JVAR
      END SUBROUTINE WORK_SAN

      SUBROUTINE DO_OMP_ISSUE
        INTEGER IVAR, JVAR

        DO 100 IVAR = 1,10
!$OMP     DO
          DO 100 JVAR = 1,10
            CALL WORK_SAN(IVAR,JVAR)
100     CONTINUE
!$OMP   ENDDO
      END SUBROUTINE DO_OMP_ISSUE

However, after that recent change, this no longer fails - it actually compiles. It fails as expected with nvfortran and gfortran.

[scamp]$ nvfortran test.F90 -c -fopenmp
NVFORTRAN-S-0155-ENDDO must immediately follow a DO loop  (test.F90: 13)
  0 inform,   0 warnings,   1 severes, 0 fatal for do_omp_issue
[scamp]$ gfortran test.F90 -c -fopenmp
test.F90:10:28:

   10 |           DO 100 JVAR = 1,10
      |                            1
Warning: Fortran 2018 deleted feature: Shared DO termination label 100 at (1)
test.F90:13:13:

   13 | !$OMP   ENDDO
      |             1
Error: Unexpected !$OMP END DO statement at (1)
[scamp]$ flang test.F90 -c -fopenmp
[scamp]$

Prior to this change, we would get a warning in line with:

./test.F90:13:9: error: The ENDDO directive must follow the DO loop associated with the loop construct
  !$OMP   ENDDO

[kkwli]

In the original test, both outer and inner do loop terminates at 100 end do. The worksharing DO directive is specified on the inner loop but the corresponding END DO directive is specified after the outer loop terminates. It looks like a mismatch to me.

[kparzysz]

Is this similar to the issue with one of the gfortran tests that was recently disabled? I don't believe that there is an OpenMP version specified in the run line for that test. If they are related, a fix for this might impact that as well.

Yes, that's pretty much the same thing. I missed the restriction in the OpenMP specs that if the END DO directive is present then the whole OpenMP construct must contain all the loops that share the terminating statement. This requirement is there until 5.0, but it's gone in 5.1+:

[5.0:97:1-4]

The loops associated with a loop-associated directive have canonical loop form if each of them is a do-loop that is a do-construct or an inner-shared-do-construct as defined by the Fortran standard. If an end do directive follows a do-construct in which several loop statements share a DO termination statement, then the directive can only be specified for the outermost of these DO statements.

[5.1:118:2-8]

DO [ label ] var = lb , ub [ , incr ]
  [intervening-code]
  loop-body
  [intervening-code]
[ label ] END DO

If the loop-nest is a nonblock-do-construct, it is treated as a block-do-construct for each DO construct.

[kparzysz]

In the original test, both outer and inner do loop terminates at 100 end do. The worksharing DO directive is specified on the inner loop but the corresponding END DO directive is specified after the outer loop terminates. It looks like a mismatch to me.

Just for the sake of argument---by the same logic, the shared terminator is a part of both loops, so it will be contained in any construct that contains the inner loop, whether there is an explicit end directive or not. ¯\_(ツ)_/¯

[kkwli]

In the original test, both outer and inner do loop terminates at 100 end do. The worksharing DO directive is specified on the inner loop but the corresponding END DO directive is specified after the outer loop terminates. It looks like a mismatch to me.

Just for the sake of argument---by the same logic, the shared terminator is a part of both loops, so it will be contained in any construct that contains the inner loop, whether there is an explicit end directive or not. ¯_(ツ)_/¯

I think the intent of the current wording to treat the following:

do 100 i=...
!$omp do
  do 100 j=...
    ...
100 continue
!$omp end do

as

do i=...
!$omp do
  do j=...
      ...
  enddo
enddo
!$omp end do

[kparzysz]

Should we summon @dreachem or @trws then?

[kparzysz]

What makes me think that this should be ok in later specs is that, for example, 5.2 says (in the syntax chapter) that an end-directive is optional or required, but nowhere does it say that it could be prohibited. Together with the treatment of label-DO loops as block-DO loops, this leads me to think that any explicit end-directive is interpreted as enclosing the corresponding loop.

I could be convinced otherwise, but it just seems strange to me that we'd break the begin-end symmetry in construct syntax, and disallow it even when it could be interpreted correctly.