[sleef] Disable COMPILER_SUPPORTS_OPENMP on Linux

[LilyWangLL]

Fixes #32360, disable option COMPILER_SUPPORTS_OPENMP on Linux, fix build error of rubberband[cli,core]:x64-linux:

/usr/bin/ld: /home/user/lily/0717/installed/x64-linux/debug/lib/libsleefdft.a(dftcommon.c.o): in function `initPlanMapLock':
/home/user/lily/0703/buildtrees/sleef/src/3.5.1-7f3a2e645a.clean/src/dft/dftcommon.c:183: undefined reference to `GOMP_critical_start'
/usr/bin/ld: /home/user/lily/0703/buildtrees/sleef/src/3.5.1-7f3a2e645a.clean/src/dft/dftcommon.c:187: undefined reference to `omp_init_lock'
/usr/bin/ld: /home/user/lily/0703/buildtrees/sleef/src/3.5.1-7f3a2e645a.clean/src/dft/dftcommon.c:183: undefined reference to `GOMP_critical_end'
/usr/bin/ld: /home/user/lily/0717/installed/x64-linux/debug/lib/libsleefdft.a(dftcommon.c.o): in function `PlanManager_loadMeasurementResultsP':
/home/user/lily/0703/buildtrees/sleef/src/3.5.1-7f3a2e645a.clean/src/dft/dftcommon.c:330: undefined reference to `omp_set_lock'
/usr/bin/ld: /home/user/lily/0703/buildtrees/sleef/src/3.5.1-7f3a2e645a.clean/src/dft/dftcommon.c:337: undefined reference to `omp_unset_lock'
/usr/bin/ld: /home/user/lily/0703/buildtrees/sleef/src/3.5.1-7f3a2e645a.clean/src/dft/dftcommon.c:354: undefined reference to `omp_unset_lock'
/usr/bin/ld: /home/user/lily/0717/installed/x64-linux/debug/lib/libsleefdft.a(dftcommon.c.o): in function `PlanManager_saveMeasurementResultsP':
/home/user/lily/0703/buildtrees/sleef/src/3.5.1-7f3a2e645a.clean/src/dft/dftcommon.c:365: undefined reference to `omp_set_lock'
/usr/bin/ld: /home/user/lily/0703/buildtrees/sleef/src/3.5.1-7f3a2e645a.clean/src/dft/dftcommon.c:371: undefined reference to `omp_unset_lock'
/usr/bin/ld: /home/user/lily/0703/buildtrees/sleef/src/3.5.1-7f3a2e645a.clean/src/dft/dftcommon.c:386: undefined reference to `omp_unset_lock'

I have submitted an issue on upstream: shibatch/sleef#467.

• Changes comply with the maintainer guide
• SHA512s are updated for each updated download
• The "supports" clause reflects platforms that may be fixed by this new version
• Any fixed CI baseline entries are removed from that file.
• Any patches that are no longer applied are deleted from the port's directory.
• The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
• Only one version is added to each modified port's versions file.

[BillyONeal]

It seems like the right fix is to add the -fopenmp switch for these rather than trying to disable entirely?

[LilyWangLL]

It seems like the right fix is to add the -fopenmp switch for these rather than trying to disable entirely?

I tried do as the following, rubberband uses Meson compiler, I added the -fopenmp compile option, but the build still fails. I'm not sure whether I need to add openmp as a dependency.

elif fft == 'sleef'
  if sleefdft_dep.found() and sleef_dep.found()
    config_summary += { 'FFT': 'SLEEF' }
    message('For FFT: using SLEEF')
    pkgconfig_requirements += sleefdft_dep
    pkgconfig_requirements += sleef_dep
  else 
    sleefdft_dep = cpp.find_library('sleefdft',
                                    dirs: get_option('extra_lib_dirs'),
                                    has_headers: ['sleefdft.h'],
                                    header_args: extra_include_args,
                                    required: true)
    sleef_dep = cpp.find_library('sleef',
                                 dirs: get_option('extra_lib_dirs'),
                                 has_headers: ['sleef.h'],
                                 header_args: extra_include_args,
                                 required: true)
    config_summary += { 'FFT': 'SLEEF' }
  endif
  feature_dependencies += sleefdft_dep
  feature_dependencies += sleef_dep
  feature_defines += ['-DHAVE_SLEEF', '-fopenmp']

[BillyONeal]

@Neumann-A @dg0yt Any ideas on anyone who understands the right way to do this in meson?

[Neumann-A]

@BillyONeal : https://stackoverflow.com/questions/45465892/how-to-use-openmp-in-a-c-project-built-with-meson

project('openmp_with_meson', 'c')
omp = dependency('openmp')
exe = executable('some_exe', 'src/main.c',
                 dependencies : omp)

However, it is probably controlled by a per project option and if not that is a problematic optional dependency. meson in vcpkg currently does not control optional dependencies in meson if the project does not give an option for it since the only way to control those optional deps is by wrapping the whole project in its own meson.build which sets the variable of the dependency call to a disabled object.

(answered a different question but whatever if rubberband provides pkg-config it should be in the pc file)

[LilyWangLL]

I also submitted an issue on upstream of rubberband: breakfastquay/rubberband#90.

[dg0yt]

My first thought was that -fopenmp doesn't belong to "...defines". And we need to fix a linker problem, not a compiler problem.

My second thought was that we would actually need an exported usage requirement for lib sleefdft. But the sleef package installs a pc file for only lib sleef.

So we are back to patching port rubberband. Based on @Neumann-A's hint, and acknowledging that we see a linking problem, the following fixes the build for me:

   feature_dependencies += sleefdft_dep
   feature_dependencies += sleef_dep
   feature_defines += ['-DHAVE_SLEEF']
+  omp_dep = dependency('openmp')
+  if omp_dep.found()
+    feature_dependencies += omp_dep.partial_dependency(link_args: true, links: true)
+  endif

[cannam]

(Rubber Band developer, coming in because of the linked issue there)

My second thought was that we would actually need an exported usage requirement for lib sleefdft. But the sleef package installs a pc file for only lib sleef.

So we are back to patching port rubberband

I don't follow the logic - if the problem is an omission in the Sleef package, why is the solution patching Rubber Band rather than Sleef?

[LilyWangLL]

   feature_dependencies += sleefdft_dep
   feature_dependencies += sleef_dep
   feature_defines += ['-DHAVE_SLEEF']
+  omp_dep = dependency('openmp')
+  if omp_dep.found()
+    feature_dependencies += omp_dep.partial_dependency(link_args: true, links: true)
+  endif

This solution resolved the build error.

[LilyWangLL]

And we need to fix a linker problem, not a compiler problem.

I agree with this. This is a linkage issue, and passing compile options can't solve the problem. I tried to pass option -fopenmp to the sleef pc file, but it didn't resolve these build errors of rubberband.

[LilyWangLL]

sleef use CMake to compile, the following codes are check openmp, it passes -fopenmp by set (CMAKE_REQUIRED_FLAGS "${OpenMP_C_FLAGS}").

if(NOT DISABLE_OPENMP)
  find_package(OpenMP)
  # Check if compilation with OpenMP really succeeds
  # It does not succeed on Travis even though find_package(OpenMP) succeeds.
  if(OPENMP_FOUND)
    set (CMAKE_REQUIRED_FLAGS "${OpenMP_C_FLAGS}")
    CHECK_C_SOURCE_COMPILES("
  #include <stdio.h>
  int main() {
  int i;
  #pragma omp parallel for
    for(i=0;i < 10;i++) { putchar(0); }
  }"
      COMPILER_SUPPORTS_OPENMP)

    CHECK_C_SOURCE_COMPILES("
  #pragma omp declare simd notinbranch
  double func(double x) { return x + 1; }
  double a[1024];
  int main() {
  #pragma omp parallel for simd
    for (int i = 0; i < 1024; i++) a[i] = func(a[i]);
  }
  "
      COMPILER_SUPPORTS_OMP_SIMD)
  endif(OPENMP_FOUND)
else()
  message(STATUS "Support for OpenMP disabled by CMake option")
endif()

[Neumann-A]

So just pass -DDISABLE_OPENMP=TRUE to the sleef build ?

[cannam]

sleef use CMake to compile, the following codes are check openmp, it passes -fopenmp by set (CMAKE_REQUIRED_FLAGS "${OpenMP_C_FLAGS}").

Sure, but then (apparently) it doesn't declare the corresponding requirements in the .pc file(s).

This doesn't really have anything to do with Rubber Band, except that Rubber Band is the "consumer" in this case. The problem is that Sleef is built in a way that imposes requirements on the user but doesn't declare what they are.

[dg0yt]

I don't follow the logic - if the problem is an omission in the Sleef package, why is the solution patching Rubber Band rather than Sleef?

@canman This was the immediate feedback for the vcpkg issue, for a quick fix without disabling openmp. I do agree that the permanent fix must be implemented on the sleef side - that's why I talked wrote about that.
But with regard to rubberband: How is sleefdft_dep = dependency('sleefdft', version: '>= 3.3.0', required: false) expected to be provided? Is there a version of sleef which provides exported config for this lib?

I tried to pass option -fopenmp to the sleef pc file

@LilyWangLL This pc file is for lib sleef, but the link library issue is with lib sleefdft.

[cannam]

But with regard to rubberband: How is sleefdft_dep = dependency('sleefdft', version: '>= 3.3.0', required: false) expected to be provided?

I don't know. If a sleefdft.pc file existed and listed -fopenmp in both Cflags and Libs then Rubber Band's build should certainly pick that up. But there seems to be no such file in my install either. All the same I have never seen this problem, which is curious.

I tried configuring Sleef (current repo tip, on Arch Linux) with -DENFORCE_OPENMP=ON and it configures happily; the CMakeCache.txt lists both ENFORCE_OPENMP=ON and COMPILER_SUPPORTS_OPENMP=1; but there is no mention of -fopenmp in the resulting Sleef Makefile. The build succeeds, of both Sleef and a Rubber Band build using it.

I then did a quick performance test, and found no measurable difference between Rubber Band builds against Sleef using -DENFORCE_OPENMP=ON and -DDISABLE_OPENMP=ON. Rubber Band only exercises a very simple subset of transforms, so that could mean that OpenMP isn't used for those anyway, or that the Sleef build didn't do what it said wrt OpenMP usage.

(But both of those were slightly faster than FFTW - we're still only talking about a 2%-3% reduction in runtime on a time-and-pitch-shifting example with the R3 engine, but definitely outside measurement error.)

[btw wrong user tagged in previous comment - I'm @cannam, not canman]