[Bug] winGRASS 8.3.dev compilation fails with --with-openmp

hellik commented 1 year ago

Describe the bug

winGRASS 8.3.dev compilation fails with --with-openmp.

I don't see this in github osgeo4w windows CI actions.

though I see it here on my win10 OSGeo4W/Msys2 winGRASS 8.3.dev build environment, and also at Martin's dailys builds.

e.g. https://wingrass.fsv.cvut.cz/grass83/logs/log-r001bf57c8b-1/error.log

GRASS GIS 8.3.dev 001bf57c8b compilation log
--------------------------------------------------
Started compilation: Sun Feb 19 14:06:42 CEST 2023
--
Errors in:
/usr/src/grass83/lib/rst/interp_float
/usr/src/grass83/lib/gpde
/usr/src/grass83/raster/r.gwflow
/usr/src/grass83/raster/r.mfilter
/usr/src/grass83/raster/r.neighbors
/usr/src/grass83/raster/r.patch
/usr/src/grass83/raster/r.resamp.filter
/usr/src/grass83/raster/r.resamp.interp
/usr/src/grass83/raster/r.resamp.rst
/usr/src/grass83/raster/r.series
/usr/src/grass83/raster/r.series.accumulate
/usr/src/grass83/raster/r.sim/simlib
/usr/src/grass83/raster/r.sim/r.sim.water
/usr/src/grass83/raster/r.sim/r.sim.sediment
/usr/src/grass83/raster/r.slope.aspect
/usr/src/grass83/raster/r.solute.transport
/usr/src/grass83/raster/r.sun
/usr/src/grass83/raster/r.univar
/usr/src/grass83/raster3d/r3.gwflow
/usr/src/grass83/vector/v.surf.bspline
/usr/src/grass83/vector/v.surf.rst
--
In case of errors please change into the directory with error and run 'make'.
If you get multiple errors, you need to deal with them in the order they
appear in the error log. If you get an error building a library, you will
also get errors from anything which uses the library.
--
Finished compilation: Sun Feb 19 14:38:52 CEST 2023

first failing is always in lib/rst/interp_float

disabling by --with-openmp=no, compilation works.

wenzeslaus commented 1 year ago

/usr/src/grass83/lib/rst/interp_float is likely just the first directory with OpenMP it hits. There needs to be some underlying error for one of the reported dirs.

nilason commented 1 year ago

There must be some environment difference:

CI configure:

checking whether to use OpenMP... "yes"
checking for location of OpenMP includes... 
checking for location of OpenMP library... 
checking omp.h usability... yes
checking omp.h presence... yes
checking for omp.h... yes
checking for omp_get_num_threads in -lomp... no
checking for GOMP_parallel_start in -lgomp... yes
checking for x86_64-w64-mingw32-gcc option to support OpenMP... -fopenmp

fsv.cvut.cz configure:

checking whether to use OpenMP... "yes"
checking for location of OpenMP includes... 
checking for location of OpenMP library... 
checking omp.h usability... yes
checking omp.h presence... yes
checking for omp.h... yes
checking for omp_get_num_threads in -lomp... yes
checking for x86_64-w64-mingw32-gcc option to support OpenMP... -fopenmp

hellik commented 1 year ago

There must be some environment difference:

yes, there are differences

CI

https://github.com/OSGeo/grass/blob/main/.github/workflows/osgeo4w.yml https://github.com/OSGeo/grass/blob/main/.github/workflows/build_osgeo4w.sh https://github.com/OSGeo/grass/blob/main/mswindows/osgeo4w/build_osgeo4w.sh

vs. Martin's procedure:

https://github.com/OSGeo/grass/blob/main/mswindows/osgeo4w/package.sh

described here: https://trac.osgeo.org/grass/wiki/CompileOnWindows

here I'm following Martin's procedure as daily builds and releases are built that way.

I've started here just another run to post make result in the first failing

hellik commented 1 year ago

the interesting is that both procedures are based upon the same basic environments:

Msys2
OSGeo4W

nilason commented 1 year ago

Is clang present in either of them?

hellik commented 1 year ago

Is clang present in either of them?

both are done in MSYS2 MinGW 64-bit

clang may be available

grafik

but not used in both procedures yet.

hellik commented 1 year ago

https://www.msys2.org/docs/environments/

hellik commented 1 year ago

a fresh 8.3.dev checkout

--------------------------------------------------
Started compilation: Sat Mar 11 20:49:01 CET 2023
--
Errors in:
/usr/src/grass/lib/rst/interp_float
/usr/src/grass/lib/gpde
/usr/src/grass/raster/r.gwflow
/usr/src/grass/raster/r.mfilter
/usr/src/grass/raster/r.neighbors
/usr/src/grass/raster/r.patch
/usr/src/grass/raster/r.resamp.filter
/usr/src/grass/raster/r.resamp.interp
/usr/src/grass/raster/r.resamp.rst
/usr/src/grass/raster/r.series
/usr/src/grass/raster/r.series.accumulate
/usr/src/grass/raster/r.sim/simlib
/usr/src/grass/raster/r.sim/r.sim.water
/usr/src/grass/raster/r.sim/r.sim.sediment
/usr/src/grass/raster/r.slope.aspect
/usr/src/grass/raster/r.solute.transport
/usr/src/grass/raster/r.sun
/usr/src/grass/raster/r.univar
/usr/src/grass/raster3d/r3.gwflow
/usr/src/grass/vector/v.surf.bspline
/usr/src/grass/vector/v.surf.rst

my@DESKTOP-VADT8Q4 MINGW64 /usr/src/grass/lib/rst/interp_float
$ make
make lib
make[1]: Entering directory '/usr/src/grass/lib/rst/interp_float'
x86_64-w64-mingw32-gcc -shared -o /usr/src/grass/dist.x86_64-w64-mingw32/lib/libgrass_interpfl.8.3.dll -L/usr/src/grass/dist.x86_64-w64-mingw32/lib -L/usr/src/grass/dist.x86_64-w64-mingw32/lib -Wl,--export-dynamic,--enable-runtime-pseudo-reloc  -L/c/osgeo4w/lib   OBJ.x86_64-w64-mingw32/distance.o OBJ.x86_64-w64-mingw32/func2d.o OBJ.x86_64-w64-mingw32/init2d.o OBJ.x86_64-w64-mingw32/input2d.o OBJ.x86_64-w64-mingw32/interp2d.o OBJ.x86_64-w64-mingw32/matrix.o OBJ.x86_64-w64-mingw32/minmax.o OBJ.x86_64-w64-mingw32/output2d.o OBJ.x86_64-w64-mingw32/point2d.o OBJ.x86_64-w64-mingw32/resout2d.o OBJ.x86_64-w64-mingw32/ressegm2d.o OBJ.x86_64-w64-mingw32/secpar2d.o OBJ.x86_64-w64-mingw32/segmen2d.o OBJ.x86_64-w64-mingw32/segmen2d_parallel.o OBJ.x86_64-w64-mingw32/vinput2d.o OBJ.x86_64-w64-mingw32/write2d.o -lgrass_gis.8.3 -lintl -lgrass_raster.8.3 -lgrass_vector.8.3 -lgrass_gmath.8.3 -lgrass_dbmiclient.8.3 -lgrass_dbmibase.8.3  -lgrass_bitmap.8.3 -lgrass_qtree.8.3 -lgrass_interpdata.8.3   -lomp
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: warning: --export-dynamic is not supported for PE+ targets, did you mean --export-all-symbols?
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: OBJ.x86_64-w64-mingw32/segmen2d_parallel.o: in function `IL_interp_segments_2d_parallel._omp_fn.0':
C:/msys64/usr/src/grass/lib/rst/interp_float/segmen2d_parallel.c:108: undefined reference to `GOMP_loop_nonmonotonic_dynamic_start'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:/msys64/usr/src/grass/lib/rst/interp_float/segmen2d_parallel.c:143: undefined reference to `GOMP_loop_nonm
onotonic_dynamic_next'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:/msys64/usr/src/grass/lib/rst/interp_float/segmen2d_parallel.c:143: undefined reference to `GOMP_loop_end'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:/msys64/usr/src/grass/lib/rst/interp_float/segmen2d_parallel.c:384: undefined reference to `GOMP_critical_
start'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:/msys64/usr/src/grass/lib/rst/interp_float/segmen2d_parallel.c:384: undefined reference to `GOMP_critical_
end'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: OBJ.x86_64-w64-mingw32/segmen2d_parallel.o: in function `IL_interp_segments_2d_parallel':
C:/msys64/usr/src/grass/lib/rst/interp_float/segmen2d_parallel.c:108: undefined reference to `GOMP_parallel'
collect2.exe: error: ld returned 1 exit status
make[1]: *** [../../../include/Make/Shlib.make:16: /usr/src/grass/dist.x86_64-w64-mingw32/lib/libgrass_interpfl.8.3.dll] Error 1
make[1]: Leaving directory '/usr/src/grass/lib/rst/interp_float'
make: *** [Makefile:15: default] Error 2

nilason commented 1 year ago

Could you share the config.log file?

hellik commented 1 year ago

Could you share the config.log file?

here attached

config_GOMP_parallel_issue_wingrass8.3dev.log

hellik commented 1 year ago

in pure Mingw64 shell

$ echo $PATH
/mingw64/bin:/usr/local/bin:/usr/bin:/bin:/c/Windows/System32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0/:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl

in a Mingw64 shell modified by the build scripts:

$ echo $PATH
/mingw64/bin:/usr/local/bin:/usr/bin:/bin:/c/Windows/System32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0/:/usr/bin/site_

the needed C:/msys64/mingw64/bin is in $PATH

nilason commented 1 year ago

Please try with deleting the line:

https://github.com/OSGeo/grass/blob/4f1305d69118cfabb29898a363bf4bd2f4fc2612/mswindows/osgeo4w/package.sh#L136

libgomp-1.dll is already included a couple of lines before and should be enough. This could be the cause of configure picking up the -lomp flag which leads to the build (linking) failure. -lgomp is the flag for gcc.

hellik commented 1 year ago

@landam @ninsbl could you try @nilason suggestions?

not sure I'm able to test it in the next days.

hellik commented 1 year ago

find attached the config log after removing /mingw64/bin/libomp.dll from package.sh

config_try_to_fix_osgeo4w_omp_issue.log

configure looks now like:

checking whether to use OpenMP... "yes"
checking for location of OpenMP includes...
checking for location of OpenMP library...
checking omp.h usability... yes
checking omp.h presence... yes
checking for omp.h... yes
checking for omp_get_num_threads in -lomp... yes
checking for x86_64-w64-mingw32-gcc option to support OpenMP... -fopenmp

CI configure (copy/paste from above):

checking whether to use OpenMP... "yes"
checking for location of OpenMP includes... 
checking for location of OpenMP library... 
checking omp.h usability... yes
checking omp.h presence... yes
checking for omp.h... yes
checking for omp_get_num_threads in -lomp... no
checking for GOMP_parallel_start in -lgomp... yes
checking for x86_64-w64-mingw32-gcc option to support OpenMP... -fopenmp

still not the same.

(no chance to compile for the moment)

nilason commented 1 year ago

The package mingw-w64-x86_64-openmp listed in:

https://trac.osgeo.org/grass/wiki/CompileOnWindows#InstalltheMSYS2environment

is a clang (llvm) package https://packages.msys2.org/base/mingw-w64-openmp. This is possibly found by configure.

nilason commented 1 year ago

Just tested without 'mingw-w64-x86_64-openmp' package in MSYS2 env.

Try again after removing the package: pacman -R mingw-w64-x86_64-openmp

hellik commented 1 year ago

The package mingw-w64-x86_64-openmp listed in:

https://trac.osgeo.org/grass/wiki/CompileOnWindows#InstalltheMSYS2environment

is a clang (llvm) package https://packages.msys2.org/base/mingw-w64-openmp. This is possibly found by configure.

AFAIU mingw-w64-x86_64-openmp is based upon and implemented on Upstream: https://openmp.llvm.org/, though it is part of the mingw64 tool chain; installed by pacman -S mingw-w64-x86_64-openmp

the clang MSys2 version is installed via pacman -S mingw-w64-clang-x86_64-openmp

the LLVM OpenMP Library is the only one available in Msys2.

though looking at the CI osgeo4w.yml, there is no pacman install of openmp. additionally in CI openmp is activated in configure.

@ninsbl any idea why the difference? is openmp working in CI winGRASS?

@landam and why it is working in winGRASS 8.2. daily builds (compiled in the same way as 8.3.dev). looking in the 8.2.daily build log there

checking whether to use OpenMP... "yes"
checking for location of OpenMP includes... 
checking omp.h usability... yes
checking omp.h presence... yes
checking for omp.h... yes
checking for location of OpenMP library... 
checking for GOMP_parallel_start... no
checking for GOMP_parallel_start in -lgomp... yes

so what have been changed between 8.2. and 8.3. regarding openmp?

anyone any idea?

hellik commented 1 year ago

Just tested without 'mingw-w64-x86_64-openmp' package in MSYS2 env.

Try again after removing the package: pacman -R mingw-w64-x86_64-openmp

did this work for you?

nilason commented 1 year ago

Just tested without 'mingw-w64-x86_64-openmp' package in MSYS2 env. Try again after removing the package: pacman -R mingw-w64-x86_64-openmp

did this work for you?

Yes. :-)

nilason commented 1 year ago

so what have been changed between 8.2. and 8.3. regarding openmp?

anyone any idea?

MSYS2's GCC supports OpenMP by default. The GRASS configure has been updated #2692 to detect clang (-lomp etc.), that is the only news for 8.3 (not yet backported for testing purposes). Before (and still for 8.2) 'mingw-w64-x86_64-openmp' was/is just ignored.

nilason commented 1 year ago

Just tested without 'mingw-w64-x86_64-openmp' package in MSYS2 env. Try again after removing the package: pacman -R mingw-w64-x86_64-openmp

did this work for you?

Yes. :-)

To be precise, I never installed it in the first place.

hellik commented 1 year ago

Installation:

pacman -S mingw-w64-x86_64-openmp

File: https://mirror.msys2.org/mingw/mingw64/mingw-w64-x86_64-openmp-15.0.7-3-any.pkg.tar.zst

looking into this file:

mingw-w64-x86_64-openmp-15.0.7-3-any.pkg\mingw64\bin\libomp.dll
mingw-w64-x86_64-openmp-15.0.7-3-any.pkg\mingw64\include\omp.h
mingw-w64-x86_64-openmp-15.0.7-3-any.pkg\mingw64\include\omp_lib.h
mingw-w64-x86_64-openmp-15.0.7-3-any.pkg\mingw64\include\omp_lib.mod
mingw-w64-x86_64-openmp-15.0.7-3-any.pkg\mingw64\include\omp_lib_kinds.mod
mingw-w64-x86_64-openmp-15.0.7-3-any.pkg\mingw64\lib\libomp.a
mingw-w64-x86_64-openmp-15.0.7-3-any.pkg\mingw64\lib\libomp.dll.a

hellik commented 1 year ago

so what have been changed between 8.2. and 8.3. regarding openmp? anyone any idea?

MSYS2's GCC supports OpenMP by default. The GRASS configure has been updated #2692 to detect clang (-lomp etc.), that is the only news for 8.3 (not yet backported for testing purposes). Before (and still for 8.2) 'mingw-w64-x86_64-openmp' was/is just ignored.

so there was a change between 8.2 and 8.3. which causes this issue.

there was a reason why we added all the dll stuff in package.sh. maybe we find out that reason. ;-) .... I guess it has to do with the well known MS windows dll hell ...

hellik commented 1 year ago

Just tested without 'mingw-w64-x86_64-openmp' package in MSYS2 env.

Try again after removing the package: pacman -R mingw-w64-x86_64-openmp

after pacman -R mingw-w64-x86_64-openmp, in MSys2 there is only available:

C:\msys64\mingw64\bin\libgomp-1.dll

no libomp.dll anymore.

nilason commented 1 year ago

Just tested without 'mingw-w64-x86_64-openmp' package in MSYS2 env. Try again after removing the package: pacman -R mingw-w64-x86_64-openmp

after pacman -R mingw-w64-x86_64-openmp, in MSys2 there is only available:

C:\msys64\mingw64\bin\libgomp-1.dll

no libomp.dll anymore.

Just what we wanted.

hellik commented 1 year ago

Just tested without 'mingw-w64-x86_64-openmp' package in MSYS2 env. Try again after removing the package: pacman -R mingw-w64-x86_64-openmp

after pacman -R mingw-w64-x86_64-openmp, in MSys2 there is only available: C:\msys64\mingw64\bin\libgomp-1.dll no libomp.dll anymore.

Just what we wanted.

now configure ouputs in 8.3.dev

checking for location of OpenMP includes...
checking for location of OpenMP library...
checking omp.h usability... yes
checking omp.h presence... yes
checking for omp.h... yes
checking for omp_get_num_threads in -lomp... no
checking for GOMP_parallel_start in -lgomp... yes
checking for x86_64-w64-mingw32-gcc option to support OpenMP... -fopenmp

it seems to look ok now.

@nilason thanks for helping to sort this out.

@landam we have to adapt the build/compile recipes for winGRASS8.3. and if the changes should be backported to 8.2, then we have to change the recipes also there.

nilason commented 1 year ago

Two steps need to be taken:

Remove line https://github.com/OSGeo/grass/blob/4f1305d69118cfabb29898a363bf4bd2f4fc2612/mswindows/osgeo4w/package.sh#L136
Delete mingw-w64-x86_64-openmp from https://trac.osgeo.org/grass/wiki/CompileOnWindows#InstalltheMSYS2environment

No 1 may be backported directly, regardless of configure backport.

hellik commented 1 year ago

Two steps need to be taken:

1. Remove line
   https://github.com/OSGeo/grass/blob/4f1305d69118cfabb29898a363bf4bd2f4fc2612/mswindows/osgeo4w/package.sh#L136

should be tackled by hopefully proper PR ;-) #2887

Delete mingw-w64-x86_64-openmp from

done

hellik commented 1 year ago

@landam could you update your msys2 build environment by pacman -R mingw-w64-x86_64-openmp?

landam commented 1 year ago

@landam could you update your msys2 build environment by pacman -R mingw-w64-x86_64-openmp?

Done.

OSGeo / grass

[Bug] winGRASS 8.3.dev compilation fails with --with-openmp #2885