Use fftw in supernova on macos #4583

dyfer · 2019-09-18T23:04:27Z

Purpose and Motivation

In the process of trying to find solutions to #4289, it was suggested to use the FFTW library on macOS for supernova.

This PR adds build instructions for including FFTW in supercollider built on macOS (for supernova only; scsynth still uses vDSP).

In order to provide static FFTW I am using a custom homebrew formula. This formula is temporarily hosted on my github, but if we decide to accept this PR, it should be moved to a more permanent place (and appropriate portions of this PR should be updated).

EDIT: This uses a regular FFTW from homebrew. Thanks to @brianlheim for figuring out how to fix linking with it.

Types of changes

Documentation
Bug fix

To-do list

Code is tested
Updated documentation
This PR is ready for review
commit history is cleaned up

dyfer · 2019-09-18T23:05:46Z

README_MACOS.md

@@ -72,7 +72,9 @@ Build instructions
    cd build
    cmake -G Xcode -DCMAKE_PREFIX_PATH=`brew --prefix qt5`  ..
    # or, if you want to build with supernova:
+    export FFTW3_DIR=`brew --prefix fftw-sc` # indicate where our custom FFTW is


alternatively, we could add /usr/local/opt/fftw-sc to search paths in cmake_modules/FindFFTW3f.cmake

dyfer · 2019-09-18T23:41:35Z

I'm not sure how to set up travis... I'd appreciate any help with that.

sonoro1234 · 2019-09-28T13:19:50Z

travis error seems related to linking with fftw. Perhaps linking should be updated in cmake?

sonoro1234 · 2019-09-28T13:22:08Z

server/scsynth/CMakeLists.txt

@@ -193,7 +193,7 @@ if (LIBSCSYNTH)
 	set_property(TARGET libscsynth PROPERTY SOVERSION   1)
 endif()

-if (FFTW3F_FOUND)
+if (FFTW3F_FOUND AND NOT APPLE)
 	target_include_directories(libscsynth PUBLIC ${FFTW3F_INCLUDE_DIR})
 	target_link_libraries(libscsynth ${FFTW3F_LIBRARY})


This should be added in APPLE and supernova for linking to succed!!

For scsynth we want to use vDSP with Apple by default, not FFTW. But I agree that it seems that now it's not possible to use FFTW if one wishes to do so...

Supernova seems to have this declaration, am I seeing this right?

I was confused with the linking error

"_rffts", referenced from: scfft_dofft(scfft*) in libsupernova.a(SC_fftlib.o) "_riffts", referenced from: scfft_doifft(scfft*) in libsupernova.a(SC_fftlib.o)

which seems related to https://github.com/supercollider/supercollider/blob/develop/common/SC_fftlib.cpp#L412
meaning that SC_FFT_GREEN is defined (and it should not)
The only cause for this seems to be FFTW is not found!!
supernova/CMakeLists.txt has merge conflicts so it is not working as expected!!

Ah thanks for the pointers @sonoro1234 ! Very much appreciated. I'll look into this...

nhthn · 2019-12-15T21:59:35Z

hi marcin, where are we at with this?

dyfer · 2019-12-16T20:22:44Z

hi marcin, where are we at with this?

Good question.
Aside from linking issues, I'm not sure what's the best way to proceed here. The main problem is that our vDSP implementation is not thread-safe. But that seems fixable, although beyond my skillset I think. This attempted to be a more-or-less temporary fix by using FFTW with supernova. That requires number of moving parts, including a custom homebrew formula. To move forward, I can see following options:

finish this PR, which (beside fixing linking errors) should probably include adding custom FFTW homebrew formula to SC repository - and then for the time being use FFTW in supernova on macOS
make vDSP implementation thread-safe

Option 1 seems more feasible, but it looks quite messy (CMake and linking-wise) to use one FFT library in supernova and another everywhere else. Ideas?

mossheim · 2019-12-22T22:31:07Z

just looking this over now. @dyfer i know we discussed this before, but remind me what is wrong with using the FFTW library supplied by homebrew right now? (https://formulae.brew.sh/formula/fftw)

i think we can definitely go ahead with this PR, or at least this general approach for fixing the issue, but i can't review this further until that's addressed.

dyfer · 2020-01-06T23:16:21Z

just looking this over now. @dyfer i know we discussed this before, but remind me what is wrong with using the FFTW library supplied by homebrew right now? (https://formulae.brew.sh/formula/fftw)

Great question, I had to dig back in as I completely forgot what the problem is.

it makes supernova (or whatever uses FFTW) depend on libgcc, which is I believe because of FFTW's fortran dependency (disabled in my formula):

cmake --build . --target install --config RelWithDebInfo
# ...
-- executable file 5: /Volumes/data/Dokumenty/src/supercollider/build/Install/SuperCollider/SuperCollider.app/Contents/Resources/supernova
-- verified='0'
-- info='external prerequisites found:
f='/Volumes/data/Dokumenty/src/supercollider/build/Install/SuperCollider/SuperCollider.app/Contents/Resources/supernova'
external_prereqs='/usr/local/lib/gcc/9/libgcc_s.1.dylib'

the FFTW from homebrew is built as a shared library, which is disabled in my formula (@brianlheim on Slack: if fftw is going to be on the critical path, it should definitely be statically linked)
homebrew builds double and long-double precision versions in addition to single precision, but we're only using the latter, so my version builds only that

I have a feeling I should mention these^^ somewhere for future reference. Where would be a good place?

i think we can definitely go ahead with this PR, or at least this general approach for fixing the issue, but i can't review this further until that's addressed.

sounds good, I'll keep working on this

dyfer · 2020-01-07T00:26:46Z

All right, so this seems to build fine now. I moved FFT definitions from server/CMakeLists to server/supernova/CMakeLists and server/scsynth/CMakeLists

I think now the next steps are:

move fftw-sc homebrew formula to a permanent place (under supercollider GH organization?)
update paths to that formula in this PR

EDIT: one outstanding question I have is whether my handling of custom fftw path is best:

 export FFTW3_DIR=`brew --prefix fftw-sc`

vs adding /usr/local/opt/fftw-sc to search paths in cmake_modules/FindFFTW3f.cmake

common/SC_fftlib.cpp

dyfer · 2020-01-07T01:29:31Z

few more thoughts on fftw build:

we could make a bottle for it (it takes quite some time to build on the CI)
- the precompiled binary would need to be hosted somewhere
I also noticed that libsndfile for Windows (for CI) is also hosted outside of SC organization so I guess there's no "official" way for hosting external libraries when needed (?)

dyfer · 2020-01-23T23:43:05Z

Following up a discussion on a dev call, @brianlheim suggested that using regular homebrew-provided ffftw library and fixing linking would be better.

I briefly looked at this again, and with my very basic understanding of linking I have a question: are we sure that we need to have libgcc_s.1.dylib ~~statically linked? Or~~ included in the bundle?

mossheim · 2020-02-16T04:03:41Z

so, having taken some time to investigate this more, i strongly feel the self-hosted repository is unnecessary, and that we can use the one from homebrew. specifically, we can use the shared library, which is also used on linux and possibly Windows (i'm too confused by that process to know for sure). i apologize for typing without thinking, and also for the wasted effort and inconvenience that may have caused you @dyfer. also, it should be trivial to have nested dependencies like libgcc_s.1.dylib copied into the final bundle, just like any other recursive dependency

i say "should" because this particular case (libfftw3f.3.dylib depending on libgcc_s.1.dylib) seems to be a bug in macdeployqt. when run under -verbose=3 it seems from the log like it simply misses the actual path that's stored in the loader info in libfftw3f.3.dylib, probably getting confused by symlinks (/usr/local/lib/gcc/9/libgcc_s.1.dylib1 is what's written in the binary, but what that actually resolves to is /usr/local/Cellar/gcc/9.2.0_3/lib/gcc/9. note that there is also a symlink at /usr/local/opt/gcc pointing to /usr/local/Cellar/gcc/9.2.0_3. i'm considering reporting this upstream):

Log: Using otool:
Log:  inspecting "/Users/brianheim/git/sc2/build/Install/SuperCollider/SuperCollider.app/Contents/Frameworks//libfftw3f.3.dylib"
Log: Using otool:
Log:  inspecting "/usr/local/lib/gcc/9/libgcc_s.1.dylib"
Log: Adding framework:
Log: Framework name "libgcc_s.1.dylib"
 Framework directory "/usr/local/lib/gcc/9/"
 Framework path "/usr/local/lib/gcc/9/libgcc_s.1.dylib"
 Binary directory "/usr/local/lib/gcc/9/"
 Binary name "libgcc_s.1.dylib"
 Binary path "/usr/local/lib/gcc/9/libgcc_s.1.dylib" ****** this is what's actually in the fftw lib loader section, what we want to replace
 Version ""
 Install name "/usr/local/opt/gcc/lib/gcc/9/libgcc_s.1.dylib"
 Deployed install name "@executable_path/../Frameworks/libgcc_s.1.dylib"
 Source file Path "/usr/local/lib/gcc/9/libgcc_s.1.dylib"
 Framework Destination Directory (relative to bundle) "Contents/Frameworks/"
 Binary Destination Directory (relative to bundle) "Contents/Frameworks/"

Log: Using install_name_tool:
Log:  in "/Users/brianheim/git/sc2/build/Install/SuperCollider/SuperCollider.app/Contents/Frameworks//libfftw3f.3.dylib"
Log:  change reference "/usr/local/opt/gcc/lib/gcc/9/libgcc_s.1.dylib" **** not a match
Log:  to "@loader_path/libgcc_s.1.dylib" ***** this would be the right replacement, but nothing happens
Log: Using install_name_tool:
Log:  in "/Users/brianheim/git/sc2/build/Install/SuperCollider/SuperCollider.app/Contents/Frameworks//libfftw3f.3.dylib"
Log:  change reference "/usr/local/Cellar/gcc/9.2.0_3/lib/gcc/9/libgcc_s.1.dylib" ***** also not a match
Log:  to "@loader_path/libgcc_s.1.dylib" **** again would be the right replacement, but no effect

**** "/usr/local/lib/gcc/9/libgcc_s.1.dylib" is never replaced

i tried to get macdeployqt to do the right thing here, for instance by adding -libpath=/usr/local/lib/gcc/9, but that didn't work. eventually, i settled on this patch. it's really a small change but i also refactored a long path to make it a little nicer. note that macdeployqt, fixup_bundle both end up eventually using install_name_tool to 'fixup' loader paths:

diff --git a/editors/sc-ide/CMakeLists.txt b/editors/sc-ide/CMakeLists.txt
index 6be0447fd..04d482eac 100644
--- a/editors/sc-ide/CMakeLists.txt
+++ b/editors/sc-ide/CMakeLists.txt
@@ -476,20 +476,29 @@ if(APPLE OR WIN32)
         # only the IDE executable is scanned normally, so force scanning of sclang as well
         # to get QtWebEngine, libsndfile, and other libs. also force scanning of QtWebEngineProcess
         # because macdeployqt is a fickle beast.
+        set(CONTENTS_DIR ${CMAKE_INSTALL_PREFIX}/SuperCollider/SuperCollider.app/Contents)
         set(DEPLOY_CMD "\"${DEPLOY_PROG}\"
                 \"${CMAKE_INSTALL_PREFIX}/SuperCollider/SuperCollider.app\"
                 -verbose=1
-                -executable=${CMAKE_INSTALL_PREFIX}/SuperCollider/SuperCollider.app/Contents/MacOS/sclang
-                -executable=${CMAKE_INSTALL_PREFIX}/SuperCollider/SuperCollider.app/Contents/Resources/scsynth
-                -executable=${CMAKE_INSTALL_PREFIX}/SuperCollider/SuperCollider.app/Contents/Resources/supernova
-                -executable=${CMAKE_INSTALL_PREFIX}/SuperCollider/SuperCollider.app/Contents/Resources/plugins/DiskIO_UGens.scx
-                -executable=${CMAKE_INSTALL_PREFIX}/SuperCollider/SuperCollider.app/Contents/Resources/plugins/DiskIO_UGens_supernova.scx
-                -executable=${CMAKE_INSTALL_PREFIX}/SuperCollider/SuperCollider.app/Contents/Frameworks/QtWebEngineCore.framework/Versions/5/Helpers/QtWebEngineProcess.app/Contents/MacOS/QtWebEngineProcess
+                -executable=${CONTENTS_DIR}/MacOS/sclang
+                -executable=${CONTENTS_DIR}/Resources/scsynth
+                -executable=${CONTENTS_DIR}/Resources/supernova
+                -executable=${CONTENTS_DIR}/Resources/plugins/DiskIO_UGens.scx
+                -executable=${CONTENTS_DIR}/Resources/plugins/DiskIO_UGens_supernova.scx
+                -executable=${CONTENTS_DIR}/Frameworks/QtWebEngineCore.framework/Versions/5/Helpers/QtWebEngineProcess.app/Contents/MacOS/QtWebEngineProcess
             ")
-        set(VERIFY_CMD "include(BundleUtilities)
+        set(VERIFY_CMD "include(BundleUtilities)")
+        if(SUPERNOVA)
+            # For supernova, libfftw3f.3.dylib has a further dependency (libgcc_s) which is copied in,
+            # but libfftw3f is not fixed up automatically when supernova is deployed.
+            string(APPEND VERIFY_CMD "
+            message(STATUS \"Fixing up bad path in libfftw3f.3.dylib manually due to faulty logic in macdeployqt\")
+            execute_process(COMMAND install_name_tool -change /usr/local/lib/gcc/9/libgcc_s.1.dylib
+                @loader_path/libgcc_s.1.dylib ${CONTENTS_DIR}/Frameworks/libfftw3f.3.dylib)")
+        endif()
+        string(APPEND VERIFY_CMD "
             message(STATUS \"Verifying app\")
-            verify_app(\"${CMAKE_INSTALL_PREFIX}/SuperCollider/SuperCollider.app\")
-            ")
+            verify_app(\"${CMAKE_INSTALL_PREFIX}/SuperCollider/SuperCollider.app\")")
     else() # WIN32
         find_program(DEPLOY_PROG windeployqt PATHS ${CMAKE_PREFIX_PATH})
         set(DEPLOY_CMD "\"${DEPLOY_PROG}\"

with this change, i am able to successfully deploy the bundle from scratch. it's a bit more low level than i'd like to be but so long as people are getting FFTW from homebrew it should work. and if they're not, then hopefully this path/symlinks issue is not a problem in the first place -- a library dependency living in /usr/local/lib is definitely treated correctly by macdeployqt.

FWIW @dyfer , FFTW3 from homebrew most definitely provides static versions of all library variants:

[brianheim@BrianMBP build]$ /bin/ls /usr/local/opt/fftw/lib/
cmake                     libfftw3_threads.3.dylib  libfftw3f_omp.a           libfftw3l_mpi.dylib
libfftw3.3.dylib          libfftw3_threads.a        libfftw3f_omp.dylib       libfftw3l_omp.3.dylib
libfftw3.a                libfftw3_threads.dylib    libfftw3f_threads.3.dylib libfftw3l_omp.a
libfftw3.dylib            libfftw3f.3.dylib         libfftw3f_threads.a       libfftw3l_omp.dylib
libfftw3_mpi.3.dylib      libfftw3f.a               libfftw3f_threads.dylib   libfftw3l_threads.3.dylib
libfftw3_mpi.a            libfftw3f.dylib           libfftw3l.3.dylib         libfftw3l_threads.a
libfftw3_mpi.dylib        libfftw3f_mpi.3.dylib     libfftw3l.a               libfftw3l_threads.dylib
libfftw3_omp.3.dylib      libfftw3f_mpi.a           libfftw3l.dylib           pkgconfig
libfftw3_omp.a            libfftw3f_mpi.dylib       libfftw3l_mpi.3.dylib
libfftw3_omp.dylib        libfftw3f_omp.3.dylib     libfftw3l_mpi.a

perhaps you weren't searching for it correctly when you tried before?

dyfer · 2020-02-16T20:53:03Z

Thank you so much @brianlheim for investigating this! And no worries about sending me down the static lib chase, I've learned some things.

with this change, i am able to successfully deploy the bundle from scratch. it's a bit more low level than i'd like to be but so long as people are getting FFTW from homebrew it should work. and if they're not, then hopefully this path/symlinks issue is not a problem in the first place -- a library dependency living in /usr/local/lib is definitely treated correctly by macdeployqt.

Great! questions:

should I apply your diff as part of this PR?
I understand that macdeploy stuff lives in sc-ide CMakeLists; is this OK or is this part of our messy CMake situation? I'd imagine that bundling a library for a server should happen independently from sc-ide? Or is the only way to get a "standalone" (libs included) server-only build by using the install target, which will always go through ide's CMakeLists anyway?

FWIW @dyfer , FFTW3 from homebrew most definitely provides static versions of all library variants:

Thanks for pointing it out. I think I was confused with the --enable-shared build option in the formula.

PS I hope you don't mind I edited your comment to make the diff blocks display properly (missing empty line after <details>)

mossheim · 2020-02-16T21:25:59Z

should I apply your diff as part of this PR?

yes, simulataneously removing all the things that have been added to work around not using homebrew FFTW

I understand that macdeploy stuff lives in sc-ide CMakeLists; is this OK or is this part of our messy CMake situation? I'd imagine that bundling a library for a server should happen independently from sc-ide? Or is the only way to get a "standalone" (libs included) server-only build by using the install target, which will always go through ide's CMakeLists anyway?

more or less by definition, the install target is the only thing that produces a redistributable artifact. and on macOS, that currently by necessity includes the IDE (although that's kind of a bug). it's fine to keep it in there for now; possibly in the future we could consider making it easier to have app packages that don't include the IDE but IIRC that isn't easy with the way CMake handles app bundles. but for right now, the IDE in macOS builds is the centerpiece executable, so it makes sense to have the installing code in that component's CMake script.

PS I hope you don't mind I edited your comment to make the diff blocks display properly (missing empty line after
)

thanks. not a problem at all, i'm very OK with editing comments if it's just to fix formatting.

dyfer · 2020-02-17T00:09:51Z

I think this works now. Notes:

I temporarily brought in changes from CMake: fix linking with makefiles+supernova+macos #4764 since I believe CI was failing because of the tests. I'll either remove this when cleaning up or rebase this PR if the other PR gets in first.
@brianlheim 's linking fix should only happen when FFTW is actually used (as opposed to the built-in "Green" FFT). I wasn't sure how to best query libsupernova target for the presence of FFTW, please review my approach...

EDIT: I rebased this PR to include changes from #4764. I also used a more streamlined check for the presence of FFTW in the macdeployqt section of CMakeLists. This is ready for (final?) review.

macOS: use FFTW in supernova

add FFTW debug posting

mossheim · 2020-04-03T02:14:57Z

editors/sc-ide/CMakeLists.txt

+                # For supernova, libfftw3f.3.dylib has a further dependency (libgcc_s) which is copied in,
+                # but libfftw3f is not fixed up automatically when supernova is deployed.
+                string(APPEND VERIFY_CMD "
+                message(STATUS \"Fixing up bad path in libfftw3f.3.dylib manually due to faulty logic in macdeployqt\")


i was going to ask why you're invoking install_name_tool directly... i forgot about this, bleh

mossheim · 2020-04-03T02:17:05Z

server/supernova/CMakeLists.txt

@@ -140,6 +160,7 @@ endif()
 if (FFTW3F_FOUND)
    target_include_directories(libsupernova PUBLIC ${FFTW3F_INCLUDE_DIR})
    target_link_libraries(libsupernova ${FFTW3F_LIBRARY})
+    set_property(TARGET libsupernova PROPERTY SUPERNOVA_FFTW ON) # for fixing linking in macdeployqt


this is a fantastic solution actually

mossheim

looks good, thanks!!

dyfer commented Sep 18, 2019

View reviewed changes

mossheim mentioned this pull request Sep 24, 2019

 #4594

Closed

4 tasks

sonoro1234 reviewed Sep 28, 2019

View reviewed changes

mossheim added comp: build CMake build system comp: supernova os: macOS labels Dec 22, 2019

dyfer force-pushed the topic/fftw-supernova-macos branch from cb6a9b4 to ea652be Compare January 7, 2020 00:02

dyfer commented Jan 7, 2020

View reviewed changes

common/SC_fftlib.cpp Show resolved Hide resolved

joshpar self-assigned this Jan 26, 2020

dyfer force-pushed the topic/fftw-supernova-macos branch 2 times, most recently from 92bcaf1 to b1c6c40 Compare February 17, 2020 09:12

dyfer added 2 commits February 17, 2020 01:15

servers: allow independent selection of FFT library

756160d

macOS: use FFTW in supernova

Update SC_fftlib.cpp

8eed3b7

add FFTW debug posting

dyfer force-pushed the topic/fftw-supernova-macos branch from b1c6c40 to 8eed3b7 Compare February 17, 2020 09:15

mossheim reviewed Apr 3, 2020

View reviewed changes

mossheim approved these changes Apr 3, 2020

View reviewed changes

mossheim merged commit 4901a25 into supercollider:develop Apr 4, 2020

mossheim mentioned this pull request Apr 11, 2020

cherry-pick 3.11.1 PRs #4865

Merged

dyfer mentioned this pull request Apr 13, 2020

FFT processing in parallel group on supernova causes loud distortion (macOS) #4289

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use fftw in supernova on macos #4583

Use fftw in supernova on macos #4583

dyfer commented Sep 18, 2019 •

edited

Loading

dyfer Sep 18, 2019

dyfer commented Sep 18, 2019

sonoro1234 commented Sep 28, 2019

sonoro1234 Sep 28, 2019 •

edited

Loading

dyfer Dec 18, 2019

sonoro1234 Dec 18, 2019 •

edited

Loading

dyfer Dec 18, 2019

nhthn commented Dec 15, 2019

dyfer commented Dec 16, 2019

mossheim commented Dec 22, 2019

dyfer commented Jan 6, 2020

dyfer commented Jan 7, 2020 •

edited

Loading

dyfer commented Jan 7, 2020

dyfer commented Jan 23, 2020 •

edited

Loading

mossheim commented Feb 16, 2020 •

edited

Loading

dyfer commented Feb 16, 2020 •

edited

Loading

mossheim commented Feb 16, 2020 •

edited

Loading

dyfer commented Feb 17, 2020 •

edited

Loading

mossheim Apr 3, 2020

mossheim Apr 3, 2020

mossheim left a comment

Use fftw in supernova on macos #4583

Use fftw in supernova on macos #4583

Conversation

dyfer commented Sep 18, 2019 • edited Loading

Purpose and Motivation

Types of changes

To-do list

dyfer Sep 18, 2019

Choose a reason for hiding this comment

dyfer commented Sep 18, 2019

sonoro1234 commented Sep 28, 2019

sonoro1234 Sep 28, 2019 • edited Loading

Choose a reason for hiding this comment

dyfer Dec 18, 2019

Choose a reason for hiding this comment

sonoro1234 Dec 18, 2019 • edited Loading

Choose a reason for hiding this comment

dyfer Dec 18, 2019

Choose a reason for hiding this comment

nhthn commented Dec 15, 2019

dyfer commented Dec 16, 2019

mossheim commented Dec 22, 2019

dyfer commented Jan 6, 2020

dyfer commented Jan 7, 2020 • edited Loading

dyfer commented Jan 7, 2020

dyfer commented Jan 23, 2020 • edited Loading

mossheim commented Feb 16, 2020 • edited Loading

dyfer commented Feb 16, 2020 • edited Loading

mossheim commented Feb 16, 2020 • edited Loading

dyfer commented Feb 17, 2020 • edited Loading

mossheim Apr 3, 2020

Choose a reason for hiding this comment

mossheim Apr 3, 2020

Choose a reason for hiding this comment

mossheim left a comment

Choose a reason for hiding this comment

dyfer commented Sep 18, 2019 •

edited

Loading

sonoro1234 Sep 28, 2019 •

edited

Loading

sonoro1234 Dec 18, 2019 •

edited

Loading

dyfer commented Jan 7, 2020 •

edited

Loading

dyfer commented Jan 23, 2020 •

edited

Loading

mossheim commented Feb 16, 2020 •

edited

Loading

dyfer commented Feb 16, 2020 •

edited

Loading

mossheim commented Feb 16, 2020 •

edited

Loading

dyfer commented Feb 17, 2020 •

edited

Loading