search

stfc / PSyclone

Domain-specific compiler and code transformation system for Finite Difference/Volume/Element Earth-system models in Fortran
BSD 3-Clause "New" or "Revised" License
104 stars 27 forks source link

Incorrect semantics of WHERE in some cases #2722

Open sergisiso opened 1 week ago

sergisiso commented 1 week ago

For this program (with #2687 merged to support non-elementals and array notation):

program where_test
    integer, dimension(10) :: a, b = 0, c = 0
    a = (/1, 2, 3, 4, 5, 6, 7, 8, 9 ,10 /)

    where(a > 5)
        b = sum(a)
        c = b(7) + 1
    elsewhere(a > 5)
        c = a
    elsewhere
        b = 1
        c = b(7) + 1
    end where

    write(*,*) c
end program

compiled with gfortran prints:

./a.out
           1           2           3           4          56          56          56          56          56          56

if I do psyclone->gfortran it prints:

./a.out
           1           2           3           4           1           1          56          56          56          56

The reason I believe is that we always create the fused version of the where bodies, but the semantics imply that each array statement is evaluated one by one for each index before moving to the next, and the fusing is just an optimisation that most of the times can be done but not always.

sergisiso commented 1 week ago

Also psyclone is happy to take non array expression like:

    where(a > 5)
        b(1) = 1
    endwhere

and cannonicalise them to some valid code with different meaning. While gfortran says:

   16 |         b(1) = 1
      |        1
Error: WHERE assignment target at (1) has inconsistent shape

I know that we assume we get valid Fortran, but since we now require to know the type of the symbols. Maybe we should check it has the valid type-rank and fail otherwise.

sergisiso commented 2 days ago

We do not need multiple statements to get it wrong, see below:

~/workspace/psyclone via 🐍 v3.12.1 (.venv)
❯ cat where_test.f90
program where_test
    integer, dimension(4) :: a = (/1,2,3,4/)
    integer, dimension(4) :: b = (/1,2,3,4/)

    where(a > 2)
        a = sum(sum(a) + b)
    end where

    write(*,*) a
end program

~/workspace/psyclone via 🐍 v3.12.1 (.venv)
❯ gfortran where_test.f90

~/workspace/psyclone via 🐍 v3.12.1 (.venv)
❯ ./a.out
           1           2          50          50

~/workspace/psyclone via 🐍 v3.12.1 (.venv)
❯ psyclone where_test.f90 | tee output.f90
program where_test
  integer, dimension(4), save :: a = (/1, 2, 3, 4/)
  integer, dimension(4), save :: b = (/1, 2, 3, 4/)
  integer :: widx1

  do widx1 = 1, 4, 1
    if (a(widx1) > 2) then
      a(widx1) = SUM(SUM(a) + b)
    end if
  enddo
  ! PSyclone CodeBlock (unsupported code) reason:
  !  - Unsupported statement: Write_Stmt
  WRITE(*, *) a

end program where_test

~/workspace/psyclone via 🐍 v3.12.1 (.venv)
❯ gfortran output.f90

~/workspace/psyclone via 🐍 v3.12.1 (.venv)
❯ ./a.out
           1           2          50         238
sergisiso commented 2 days ago

We are also currently not testing the conditional expression for UnresolvedInterfaces and Elemental Calls as we do inside the where body, e.g. for where (a < my_elemental(a)), that could be a problem.