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

Compile time constant lookup tables with cmake #438

Merged
merged 10 commits into from Nov 3, 2018
Merged

Conversation

derselbst
Copy link
Member

@derselbst derselbst commented Oct 2, 2018

Like #423 it automatically generates lookup tables. Neither does it require additional build dependencies, nor additional cmake flags. Unlike #423 it does not use the preprocessor. Instead it generates temporary files and includes them during compilation. Currently there is no fallback to runtime initialization.

The problem of this solution: How to force cmake to look for the host compiler? When cross compiling $CC usually points to the target compiler. $PATH is "polluted" with libs and binaries for the target platform as well. The only solution I came up with is to unset $CC within cmake, which seems to work at least when cross compiling for android.

Tables are only generated once. Make sure your build directory is empty (i.e. start with a clean cmake run).

Closes #423.
Closes #433.

@kcat
Copy link

kcat commented Oct 2, 2018

When cross compiling $CC usually points to the target compiler.

For cross-compiling with CMake, you should be using a toolchain file and leave the environment variables alone. Here's the cross-compile toolchain files I use for Windows and Android, for example.

@derselbst
Copy link
Member Author

I was using a toolchain file when cross compiling for android, namely the official one shipped with the NDK. But cross compiling usually means that one also needs to build depending libraries, many of them using autotools as build system. This is done by setting up environment variables, which also is the recommend way to build for android.

Also note that unset(ENV{CC}) only takes effect for cmake itself.

@mawe42
Copy link
Member

mawe42 commented Oct 3, 2018

This works fine for my buildroot-based cross compiling setup. Both with and without the unset(ENV{CC}).
The buildroot cmake system uses a toolchain file for cross-compiling (not only env variables, as I previously said on the mailing list).

@derselbst
Copy link
Member Author

@carlo-bramini: Could you please test whether this works for your use-case?

@carlo-bramini
Copy link
Contributor

I just tried to cross compile mingw32 on cygwin and it worked fine. I will try also with ARM cross compiler.

@derselbst
Copy link
Member Author

I will try also with ARM cross compiler.

Yes please, that would be of special interest for me.

@carlo-bramini
Copy link
Contributor

Actually the generated tables have the same values of the ones generated by my branch with Python support, so in my opinion it should work in the same way. It seems ok to me.

@derselbst
Copy link
Member Author

derselbst commented Nov 1, 2018 via email

@carlo-bramini
Copy link
Contributor

Actually, not really, if I lauch the cmake as it is, it breaks because it cannot find GLIB.
So, I had to import some bits of my branch that works also without GLIB support

I did these steps:

  1. I replaced fluid_sys.c, fluid_sys.h and fluidsynth_priv.h with my versions.
  2. I synced all sources not touched by your changes with the fluidsynth master, otherwire I could not do point (1)
  3. In CMakeLists.txt, I removed the "REQUIRED" keyword near GLIB detection.
  4. I configured with:
    $ cmake ../fluidsynth-autogen-ltab-cmake -DCMAKE_TOOLCHAIN_FILE=../arm.cmake -Denable-network=0 -Denable-oss=0 -Denable-readline=0
  5. Finally, make command.

I have to say that for doing this test I used GCC-4.1.1, which is older than the one I normally use.
I had to do this because I did a cross compilation with CYGWIN and since my ARM GCC-7 was complied with MINGW, it was not able to work properly under CYGWIN layer.
During the compilation with this GCC-4.1.1, I noticed messages like this one:

In function 'fluid_rvoice_buffers_mix': warning: ignoring #pragma omp simd

Perhaps it is not important also on even older versions of GCC, however I signal it .

@derselbst
Copy link
Member Author

Thanks for your explanations Carlo. The tweaks you described are absolutely fine, as they are specific to your environment. I agree that they should not impair the changes proposed by this PR. The warning is negligible and only occurs for old compilers that dont support openMP 4.0.

Will apply the changes in the next days.

@derselbst derselbst added this to the 2.0-post milestone Nov 3, 2018
@derselbst derselbst merged commit 0160543 into master Nov 3, 2018
@derselbst derselbst deleted the autogen-ltab-cmake branch November 3, 2018 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants