Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
hellik opened this issue Mar 11, 2023 · 30 comments · Fixed by hellik/grass_fix_openmp_wingrass#1 or #2887
Closed
Assignees
Labels
bug Something isn't working windows Microsoft Windows specific
Milestone

Comments

@hellik
Copy link
Member

hellik commented Mar 11, 2023

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.

@hellik hellik added bug Something isn't working windows Microsoft Windows specific labels Mar 11, 2023
@hellik hellik added this to the 8.3.0 milestone Mar 11, 2023
@wenzeslaus
Copy link
Member

/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
Copy link
Contributor

nilason commented Mar 11, 2023

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
Copy link
Member Author

hellik commented Mar 11, 2023

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
Copy link
Member Author

hellik commented Mar 11, 2023

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

  • Msys2
  • OSGeo4W

@nilason
Copy link
Contributor

nilason commented Mar 11, 2023

Is clang present in either of them?

@hellik
Copy link
Member Author

hellik commented Mar 11, 2023

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
Copy link
Member Author

hellik commented Mar 11, 2023

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

Name Prefix Toolchain Architecture C Library C++ Library
  MSYS /usr gcc x86_64 cygwin
  UCRT64 /ucrt64 gcc x86_64 ucrt
  CLANG64 /clang64 llvm x86_64 ucrt
  CLANGARM64 /clangarm64 llvm aarch64 ucrt
  CLANG32 /clang32 llvm i686 ucrt
  MINGW64 /mingw64 gcc x86_64 msvcrt
  MINGW32 /mingw32 gcc i686 msvcrt

@hellik
Copy link
Member Author

hellik commented Mar 11, 2023

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
Copy link
Contributor

nilason commented Mar 11, 2023

Could you share the config.log file?

@hellik
Copy link
Member Author

hellik commented Mar 11, 2023

Could you share the config.log file?

here attached

config_GOMP_parallel_issue_wingrass8.3dev.log

@hellik
Copy link
Member Author

hellik commented Mar 11, 2023

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
Copy link
Contributor

nilason commented Mar 12, 2023

Please try with deleting the line:

/mingw64/bin/libomp.dll

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
Copy link
Member Author

hellik commented Mar 12, 2023

@landam @ninsbl could you try @nilason suggestions?

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

@hellik
Copy link
Member Author

hellik commented Mar 12, 2023

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
Copy link
Contributor

nilason commented Mar 12, 2023

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
Copy link
Contributor

nilason commented Mar 13, 2023

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
Copy link
Member Author

hellik commented Mar 13, 2023

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
Copy link
Member Author

hellik commented Mar 13, 2023

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
Copy link
Contributor

nilason commented Mar 13, 2023

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
Copy link
Contributor

nilason commented Mar 13, 2023

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
Copy link
Contributor

nilason commented Mar 13, 2023

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
Copy link
Member Author

hellik commented Mar 13, 2023

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
Copy link
Member Author

hellik commented Mar 13, 2023

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
Copy link
Member Author

hellik commented Mar 13, 2023

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
Copy link
Contributor

nilason commented Mar 13, 2023

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
Copy link
Member Author

hellik commented Mar 13, 2023

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
Copy link
Contributor

nilason commented Mar 13, 2023

Two steps need to be taken:

  1. Remove line

    /mingw64/bin/libomp.dll

  2. 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
Copy link
Member Author

hellik commented Mar 13, 2023

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
Copy link
Member Author

hellik commented Mar 13, 2023

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

@landam
Copy link
Member

landam commented Mar 14, 2023

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

Done.

hellik added a commit that referenced this issue Mar 14, 2023
@hellik hellik self-assigned this Mar 14, 2023
ninsbl pushed a commit to ninsbl/grass that referenced this issue Mar 20, 2023
neteler pushed a commit to nilason/grass that referenced this issue Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working windows Microsoft Windows specific
Projects
None yet
4 participants