[Flang] [OpenMP] New OpenMP and Compiler directive incompatibility causes large apps buildability regression.

[scamp-nvidia]

Recently, it appears that #168884 introduced a change that caused an incompatiblity between having openmp structures and compiler directives. Consider the following:

program omp_cdir_crash
  implicit none
  integer, parameter :: n = 10
  real :: a(n)
  integer :: i

!$OMP PARALLEL DO
!dir$ anything goes here
  do i = 1, n
    a(i) = real(i)
  end do
!$OMP END PARALLEL DO

  print *, 'a(1)=', a(1), ' a(n)=', a(n)
end program omp_cdir_crash

The above compiles fine with GCC and NVHPC

scamp$ nvfortran test.F90 -o test -mp
scamp$ gfortran test.F90 -o test -fopenmp

However, after the recent change, Flang now chokes on this:

scamp$ flang test.F90 -o test -fopenmp
error: Semantic errors in test.F90
./test.F90:7:7: error: A DO loop must follow the PARALLEL DO directive
  !$OMP PARALLEL DO
        ^^^^^^^^^^^
./test.F90:8:7: warning: Unrecognized compiler directive was ignored [-Wignored-directive]
  !dir$ anything goes here
        ^^^^^^^^^^^^^^^^^^^
./test.F90:8:7: error: Compiler directives are not allowed inside OpenMP loop constructs
  !dir$ anything goes here
        ^^^^^^^^^^^^^^^^^^^

This immediately popped up in our nightly testing for some large applications used by ECWMF and MeteoFrance, along with OpenRadioss - all of which make use of compiler directives nested inside an openmp region. I'm not positive what NVHPC and Gfortran do in those situations, but I do know that they compile the code and that to support similar behavior and portability, Flang should likely compile it as well. Tagging @kiranchandramohan too for an OpenMP supporting break.

Note that I talked with some people working on OpenACC parsing things, and they highlighted they ran into a similar issue and resolved it with this sort of handling, where they move the directive prior to the parallel region, so it's preserved:

llvm-project/flang/lib/Semantics/canonicalize-acc.cpp

Line 114 in e6f60a6

void moveCompilerDirectivesBefore(

[llvmbot]

@llvm/issue-subscribers-flang-frontend

Author: None (scamp-nvidia)

Recently, it appears that #168884 introduced a change that caused an incompatiblity between having openmp structures and compiler directives. Consider the following:

program omp_cdir_crash
  implicit none
  integer, parameter :: n = 10
  real :: a(n)
  integer :: i

!$OMP PARALLEL DO
!dir$ anything goes here
  do i = 1, n
    a(i) = real(i)
  end do
!$OMP END PARALLEL DO

  print *, 'a(1)=', a(1), ' a(n)=', a(n)
end program omp_cdir_crash

The above compiles fine with GCC and NVHPC

scamp$ nvfortran test.F90 -o test -mp
scamp$ gfortran test.F90 -o test -fopenmp

However, after the recent change, Flang now chokes on this:

scamp$ flang test.F90 -o test -fopenmp
error: Semantic errors in test.F90
./test.F90:7:7: error: A DO loop must follow the PARALLEL DO directive
  !$OMP PARALLEL DO
        ^^^^^^^^^^^
./test.F90:8:7: warning: Unrecognized compiler directive was ignored [-Wignored-directive]
  !dir$ anything goes here
        ^^^^^^^^^^^^^^^^^^^
./test.F90:8:7: error: Compiler directives are not allowed inside OpenMP loop constructs
  !dir$ anything goes here
        ^^^^^^^^^^^^^^^^^^^

This immediately popped up in our nightly testing for some large applications used by ECWMF and MeteoFrance, along with OpenRadioss - all of which make use of compiler directives nested inside an openmp region. I'm not positive what NVHPC and Gfortran do in those situations, but I do know that they compile the code and that to support similar behavior and portability, Flang should likely compile it as well. Tagging @kiranchandramohan too for an OpenMP supporting break.

Note that I talked with some people working on OpenACC parsing things, and they highlighted they ran into a similar issue and resolved it with this sort of handling, where they move the directive prior to the parallel region, so it's preserved:

llvm-project/flang/lib/Semantics/canonicalize-acc.cpp

Line 114 in e6f60a6

void moveCompilerDirectivesBefore(

[kiranchandramohan]

@kparzysz Were the non-openmp directives moved out during canonicalization? And is that not happening anymore? Or were these non-openmp directives ignored previously and now the parser sees them and errors?

[kparzysz]

Previously any directives were moved to immediately after the OpenMP construct. That was, strictly speaking, a wrong thing to do, e.g. given

!$omp do
!dir$ something   ! here it's on the i-loop
do i = ...
end do
!$omp end do
do j = ...
end do

we'd end up with

!$omp do
do i = ...
end do
!$omp end do
!dir$ something  ! now applying to the j-loop
do j = ...
end do

We had an internal discussion about this in my team and given lack of clear guidance in any specs or standards we decided that for now we're going to make it an error and decide what to do with this later. It seems like "later" is now...

What we actually do in the parser is that we keep the !dir$ directives where they are (i.e. inside the OpenMP construct in the above example), just complain about them later in semantic checks. What I can do is that I can disable flagging of errors in the semantic checks. I should have a PR ready later today (CST time).