ExtremeFLOW / neko

/ᐠ. 。.ᐟ\ᵐᵉᵒʷˎˊ˗
https://neko.cfd/
Other
176 stars 32 forks source link

krylov/pc_hsmg.f90 doesn't build with flang-new #1282

Open jeffhammond opened 6 months ago

jeffhammond commented 6 months ago

flang-new is not a perfect compiler but it is rather strict about the Fortran standard for the things it supports.

Can you look at this and see the errors about non-compliance are correct?

https://github.com/jeffhammond/HPCInfo/blob/master/buildscripts/llvm-git.sh builds flang-new. Building MPICH isn't hard but I can give you the entire reproducer as a script if necessary.

/opt/mpich/llvm/bin/mpifort  -I/home/jehammond/FORTRAN/json-fortran-install/include -I/home/jehammond/vapaa/source -c -o krylov/pc_hsmg.o krylov/pc_hsmg.f90
error: Semantic errors in krylov/pc_hsmg.f90
./schwarz.mod:605:24: error: Invalid specification expression: reference to impure function 'dofmap_size'
  real(8),intent(inout)::e(1_8:int(dofmap_size(this=this%dm),kind=8))
                         ^
./schwarz.mod:605:46: error: Keyword 'this=' may not appear in a reference to a procedure with an implicit interface
  real(8),intent(inout)::e(1_8:int(dofmap_size(this=this%dm),kind=8))
                                               ^^^^
./schwarz.mod:606:24: error: Invalid specification expression: reference to impure function 'dofmap_size'
  real(8),intent(inout)::r(1_8:int(dofmap_size(this=this%dm),kind=8))
                         ^
./schwarz.mod:606:46: error: Keyword 'this=' may not appear in a reference to a procedure with an implicit interface
  real(8),intent(inout)::r(1_8:int(dofmap_size(this=this%dm),kind=8))
timofeymukha commented 6 months ago

Hi! Currently, we do not support flang. The first error is surprising to me since dofmap_size is actually pure: pure function dofmap_size(this) result(res). However, the call should probably look like int(this%dm%size(), kind=8)

njansson commented 6 months ago

flang-new is not a perfect compiler but it is rather strict about the Fortran standard for the things it supports.

Can you look at this and see the errors about non-compliance are correct?

https://github.com/jeffhammond/HPCInfo/blob/master/buildscripts/llvm-git.sh builds flang-new. Building MPICH isn't hard but I can give you the entire reproducer as a script if necessary.

/opt/mpich/llvm/bin/mpifort  -I/home/jehammond/FORTRAN/json-fortran-install/include -I/home/jehammond/vapaa/source -c -o krylov/pc_hsmg.o krylov/pc_hsmg.f90
error: Semantic errors in krylov/pc_hsmg.f90
./schwarz.mod:605:24: error: Invalid specification expression: reference to impure function 'dofmap_size'
  real(8),intent(inout)::e(1_8:int(dofmap_size(this=this%dm),kind=8))
                         ^
./schwarz.mod:605:46: error: Keyword 'this=' may not appear in a reference to a procedure with an implicit interface
  real(8),intent(inout)::e(1_8:int(dofmap_size(this=this%dm),kind=8))
                                               ^^^^
./schwarz.mod:606:24: error: Invalid specification expression: reference to impure function 'dofmap_size'
  real(8),intent(inout)::r(1_8:int(dofmap_size(this=this%dm),kind=8))
                         ^
./schwarz.mod:606:46: error: Keyword 'this=' may not appear in a reference to a procedure with an implicit interface
  real(8),intent(inout)::r(1_8:int(dofmap_size(this=this%dm),kind=8))

Finally, I managed to get a reproducer environment up and running, and I ran into the same issue.

A bit puzzled by this error since the function in question is indeed a pure TBP (unless my interpretation of the standard is wrong).

https://github.com/ExtremeFLOW/neko/blob/243c35d6e024ae35df932fa187d92c81eac93488/src/sem/dofmap.f90#L195-L199

However, the error is triggered when the TBP function is used to define the size of a dummy argument, maybe this isn't allowed?

With the following patch, Neko compiles perfectly fine with flang-new!

diff --git a/src/common/global_interpolation.F90 b/src/common/global_interpolation.F90
index 5d9e70318..aaf6a70a5 100644
--- a/src/common/global_interpolation.F90
+++ b/src/common/global_interpolation.F90
@@ -516,7 +516,7 @@ contains
   subroutine global_interpolation_evaluate(this, interp_values, field)
     class(global_interpolation_t), intent(inout) :: this
     real(kind=rp), intent(inout) :: interp_values(this%n_points)
-    real(kind=rp), intent(inout) :: field(this%dof%size())
+    real(kind=rp), intent(inout) :: field(this%dof%ntot)

 #ifdef HAVE_GSLIB
     if (.not. this%all_points_local) then
diff --git a/src/math/schwarz.f90 b/src/math/schwarz.f90
index a946d0ba0..1e6dc5f5f 100644
--- a/src/math/schwarz.f90
+++ b/src/math/schwarz.f90
@@ -377,7 +377,7 @@ contains

   subroutine schwarz_compute(this, e, r)
     class(schwarz_t), intent(inout) :: this
-    real(kind=rp), dimension(this%dm%size()), intent(inout) :: e, r
+    real(kind=rp), dimension(this%dm%ntot), intent(inout) :: e, r
     integer :: n, enx, eny, enz, ns
     real(kind=rp), parameter :: zero = 0.0_rp
     real(kind=rp), parameter :: one = 1.0_rp
diff --git a/src/sem/dofmap.f90 b/src/sem/dofmap.f90
index 16f4e0b40..9efea4427 100644
--- a/src/sem/dofmap.f90
+++ b/src/sem/dofmap.f90
@@ -56,7 +56,7 @@ module dofmap
      real(kind=rp), allocatable :: x(:,:,:,:)       !< Mapping to x-coordinates
      real(kind=rp), allocatable :: y(:,:,:,:)       !< Mapping to y-coordinates
      real(kind=rp), allocatable :: z(:,:,:,:)       !< Mapping to z-coordinates
-     integer, private :: ntot                       !< Total number of dofs
+     integer :: ntot                       !< Total number of dofs

      type(mesh_t), pointer :: msh
      type(space_t), pointer :: Xh
jeffhammond commented 6 months ago

well that's fantastic news. thanks for figuring this out. i'll try to find a language lawyer to weigh in on the TBP matter.

klausler commented 6 months ago

The module file shouldn't contain references to inaccessible PRIVATE functions, of course. (A monomorphic reference to a TBP is resolved by semantics to its bound procedure at compilation time.). Will fix; thanks for the clear description of the bug.