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

[FR] dynamic OpenMP #1028

Closed
herbakamil opened this issue Jun 28, 2019 · 26 comments
Closed

[FR] dynamic OpenMP #1028

herbakamil opened this issue Jun 28, 2019 · 26 comments
Assignees
Milestone

Comments

@herbakamil
Copy link

Is your feature request related to a problem? Please describe.
Currently OpenMP is only available statically. We had a problem that different teams prepared libraries using OpenMP and when integration team combined them in a single Android Java app they were failing because OpenMP didn't like the fact that there were two instances of it.

Describe the solution you'd like
I would like to have the flag allowing me to link openmp dynamically into my Android NDK app

Describe alternatives you've considered
We've made experiment with downloading some old dynamic version of OpenMP and linking it with our module and it even worked but it would be hard to enforce such solution on other teams.

Additional context

@stephenhines
Copy link
Collaborator

@pirama-arumuga-nainar I think that we can probably build this along with LIBOMP_ENABLE_SHARED in our build.py scripts. Do you know why we might not have enabled this (other than conforming with previous libomp.a from GCC)?

@pirama-arumuga-nainar
Copy link
Collaborator

Yes, we only included libomp.a to match gcc, and the reasoning that someone can make a shared library out of it if necessary.

But that doesn't seem to be the case as I figured when turning LIBOMP_ENABLE_SHARED in our build script. I think it's a reasonable request and will address it.

@herbakamil
Copy link
Author

herbakamil commented Jul 8, 2019 via email

@pirama-arumuga-nainar
Copy link
Collaborator

In terms of NDK releases, I'd say r22 sounds reasonable (or whenever the NDK clang gets updated past the one planned for r21).

It seems the OpenMP CMake files are not setup to simultaneously build both a static and a shared library, mainly due to limitations in the build system. For e.g., LIBOMP_LIBRARY_SUFFIX is set in openmp/runtime/CMakeLists.txt and gets used in openmp/runime/src/CMakeLists.txt as if there's either just a shared or a static lib.

I am not too familiar with CMake - so if anyone can suggest a simple way to refactor this setup, that'd be useful. Worst case, we can just build shared and static libraries in separate CMake invocations (like we do for the NDK vs. the platform).

@DanAlbert
Copy link
Member

(r22 corresponds roughly with Q4)

@pirama-arumuga-nainar
Copy link
Collaborator

I decided to just clone/duplicate another build to get the .so.
https://android-review.googlesource.com/c/toolchain/llvm_android/+/1012416 is the change. @herbakamil can you test the libraries from https://ci.android.com/builds/submitted/5715392/linux/latest/clang-5715392-linux-x86.tar.bz2?

@taki-jaro
Copy link

Can I ask how one can use it? This is whole clang compiler, should I somehow config the ndk to use this compiler instead of the bundled one?

@pirama-arumuga-nainar
Copy link
Collaborator

You can copy the libomp.so from this package adjacent to the libomp.a in an NDK's Clang directory (and maybe delete the libomp.a to ensure the shared object gets used?). I think that should mostly work since openmp runtime doesn't change that frequently.

@taki-jaro
Copy link

I can confirm that it works, but you obviously need also to instruct android studio to copy libomp.so into apk

@DanAlbert
Copy link
Member

DanAlbert commented Jul 11, 2019

For now, add it to your jniLibs like you would for ASan: https://developer.android.com/ndk/guides/asan#running

Getting Gradle fixed will take some time. 3.6 is the absolute earliest it could happen (3.5 is already quite locked down).

@taki-jaro
Copy link

I understand, I can confirm that after adding to jniLibs it indeed works.

@paleozogt
Copy link

paleozogt commented Aug 23, 2019

We've found that setting KMP_DUPLICATE_LIB_OK (see here) works around the crash. Is there any reason to think that would cause other problems?

@DanAlbert
Copy link
Member

@pirama-arumuga-nainar afaict the only way to use the static libomp now (the old behavior) is an explicit -Wl,-Bstatic -lomp -Wl,-Bdynamic. Using -fopenmp to let the driver pick its own library causes it to be linked after the other flags, so it doesn't get -Bstatic. Is that desired, or should there be a -fstatic-openmp?

@DanAlbert
Copy link
Member

DanAlbert commented Aug 23, 2019

https://android-review.googlesource.com/c/platform/ndk/+/1108820 fixes the tests, but there's more that needs to be done here:

  • Tests for the dynamic linked omp.
  • Copy libomp.so to the ndk-build install directory when appropriate.
  • Test infrastructure for copying libomp.so to the device for CMake tests.

@pirama-arumuga-nainar
Copy link
Collaborator

@DanAlbert Agree - a -fstatic-openmp in the driver is needed. I can create the driver change, but it won't be ready for r21, unfortunately.

disigma pushed a commit to wimal-build/ndk that referenced this issue Aug 23, 2019
We don't currently have any infrastructure for copying the openmp
runtime to the device as part of the test. Keep using the static
openmp like we used to.

For ndk-build, we should be copying the runtime library to the install
directory if appropriate.

For CMake, the install directory is not used. We have this same
problem with ASan, so if we decide that CMake should do anything about
that we can handle openmp the same way.

Test: ./run_tests.py --rebuild --filter openmp
Bug: android/ndk#1028
Change-Id: Id41455c7dad1117aeae4587a2bc1b9601d55e472
@DanAlbert DanAlbert added this to the r21 milestone Sep 4, 2019
dtzWill pushed a commit to llvm-mirror/clang that referenced this issue Sep 9, 2019
Summary:
For Gnu, FreeBSD and NetBSD, this option forces linking with the static
OpenMP host runtime (similar to -static-libgcc and -static-libstdcxx).

Android's NDK will start the shared OpenMP runtime in addition to the static
libomp.  In this scenario, the linker will prefer to use the shared library by
default.  Add this option to enable linking with the static libomp.

Reviewers: Hahnfeld, danalbert, srhines, joerg, jdoerfert

Subscribers: guansong, cfe-commits

Tags: #clang

Fixes android/ndk#1028

Differential Revision: https://reviews.llvm.org/D67200

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@371437 91177308-0d34-0410-b5e6-96231b3b80d8
llvm-git-migration pushed a commit to llvm/llvm-project that referenced this issue Sep 9, 2019
Summary:
For Gnu, FreeBSD and NetBSD, this option forces linking with the static
OpenMP host runtime (similar to -static-libgcc and -static-libstdcxx).

Android's NDK will start the shared OpenMP runtime in addition to the static
libomp.  In this scenario, the linker will prefer to use the shared library by
default.  Add this option to enable linking with the static libomp.

Reviewers: Hahnfeld, danalbert, srhines, joerg, jdoerfert

Subscribers: guansong, cfe-commits

Tags: #clang

Fixes android/ndk#1028

Differential Revision: https://reviews.llvm.org/D67200

llvm-svn: 371437
spurious pushed a commit to spurious/clang-mirror that referenced this issue Sep 9, 2019
Summary:
For Gnu, FreeBSD and NetBSD, this option forces linking with the static
OpenMP host runtime (similar to -static-libgcc and -static-libstdcxx).

Android's NDK will start the shared OpenMP runtime in addition to the static
libomp.  In this scenario, the linker will prefer to use the shared library by
default.  Add this option to enable linking with the static libomp.

Reviewers: Hahnfeld, danalbert, srhines, joerg, jdoerfert

Subscribers: guansong, cfe-commits

Tags: #clang

Fixes android/ndk#1028

Differential Revision: https://reviews.llvm.org/D67200

git-svn-id: http://llvm.org/svn/llvm-project/cfe/trunk@371437 91177308-0d34-0410-b5e6-96231b3b80d8
disigma pushed a commit to wimal-build/ndk that referenced this issue Sep 16, 2019
Includes the fix for 32-bit x86 with gold, and also the
`-static-openmp` driver option.

Test: ./checkbuild.py && ./run_tests.py
Bug: android/ndk#1028
Bug: android/ndk#1076
Change-Id: Ide30be0e318d861bf8491cae03e3374e1893d2bb
@DanAlbert
Copy link
Member

We have the -static-openmp flag now. All the testing things mentioned above still need to be done though.

disigma pushed a commit to wimal-build/ndk that referenced this issue Sep 17, 2019
Test: ./checkbuild.py && ./run_tests.py
Bug: android/ndk#1028
Change-Id: Ie927be61a23c3780488312bac17f7e533dc101a6
@DanAlbert
Copy link
Member

https://android-review.googlesource.com/c/platform/ndk/+/1127561 adds the test for ndk-build as well as the infrastructure for installing it. I'm going to punt on the CMake side because our test builder doesn't currently handle packaging additional libraries with CMake. It shouldn't be too hard to do, but it doesn't need to block r21.

disigma pushed a commit to wimal-build/ndk that referenced this issue Sep 24, 2019
Test: ./run_tests.py --rebuild --filter openmp
Bug: android/ndk#1028
Change-Id: Icfbcb933ca8ca03f9e3a8a36dab438cae6356fa3
disigma pushed a commit to wimal-build/ndk that referenced this issue Sep 25, 2019
Test: ./run_tests.py --filter openmp_shared
Bug: android/ndk#1028
Change-Id: I965aeb3286d1ee141d8b0157941f86e8544c81d0
jrtc27 pushed a commit to CTSRD-CHERI/llvm-project that referenced this issue Oct 5, 2019
Summary:
For Gnu, FreeBSD and NetBSD, this option forces linking with the static
OpenMP host runtime (similar to -static-libgcc and -static-libstdcxx).

Android's NDK will start the shared OpenMP runtime in addition to the static
libomp.  In this scenario, the linker will prefer to use the shared library by
default.  Add this option to enable linking with the static libomp.

Reviewers: Hahnfeld, danalbert, srhines, joerg, jdoerfert

Subscribers: guansong, cfe-commits

Tags: #clang

Fixes android/ndk#1028

Differential Revision: https://reviews.llvm.org/D67200

llvm-svn: 371437
da-x pushed a commit to da-x/llvm-project that referenced this issue Nov 19, 2019
Summary:
For Gnu, FreeBSD and NetBSD, this option forces linking with the static
OpenMP host runtime (similar to -static-libgcc and -static-libstdcxx).

Android's NDK will start the shared OpenMP runtime in addition to the static
libomp.  In this scenario, the linker will prefer to use the shared library by
default.  Add this option to enable linking with the static libomp.

Reviewers: Hahnfeld, danalbert, srhines, joerg, jdoerfert

Subscribers: guansong, cfe-commits

Tags: #clang

Fixes android/ndk#1028

Differential Revision: https://reviews.llvm.org/D67200

llvm-svn: 371437
@rpattabi
Copy link

rpattabi commented Apr 1, 2020

libomp.so is not automatically copied when checked with NDK r21 and AGP 3.6.1, which means runtime crashes.

@DanAlbert
Copy link
Member

Could you file a Studio bug? The NDK isn't responsible for building APKs. One of my commits above says we do this for ndk-build, so the rest is up to Gradle.

@rpattabi
Copy link

rpattabi commented Apr 1, 2020

Filed this issue on Android Studio.

Interestingly, static linking as suggested here, with -static-openmp linker flag does not seem to work. I get the following linker errors:

FAILED: ../build/intermediates/cmake/release/obj/armeabi-v7a/libfoo_neon.so 
  : && /opt/android/Sdk/ndk/21.0.6113669/toolchains/llvm/prebuilt/linux-x86_64/bin/clang++ --target=armv7-none-linux-androideabi21 --gcc-toolchain=/opt/android/Sdk/ndk/21.0.6113669/toolchains/llvm/prebuilt/linux-x86_64 --sysroot=/opt/android/Sdk/ndk/21.0.6113669/toolchains/llvm/prebuilt/linux-x86_64/sysroot -fPIC -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -march=armv7-a -mthumb -Wformat -Werror=format-security   -Oz -DNDEBUG  -Wl,--exclude-libs,libgcc_real.a -Wl,--exclude-libs,libatomic.a -static-libstdc++ -Wl,--build-id -Wl,--fatal-warnings -Wl,--exclude-libs,libunwind.a -Wl,--no-undefined -Qunused-arguments -shared -Wl,-soname,libfoo_neon.so -o ../build/intermediates/cmake/release/obj/armeabi-v7a/libfoo_neon.so CMakeFiles/foo_neon.dir CMakeFiles/foo_neon.dir/src/main/cpp/foo.cpp.o CMakeFiles/foo_neon.dir/src/main/cpp/foo1.cpp.o  -L/home/ragu/code/common/audio/libs/jni/armeabi-v7a -static-openmp /opt/android/Sdk/ndk/21.0.6113669/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/arm-linux-androideabi/21/liblog.so -latomic -lm && :

  ../src/main/cpp/foo.cpp:132: error: undefined reference to '__kmpc_fork_call'
  ../src/main/cpp/foo.cpp:133: error: undefined reference to '__kmpc_for_static_init_4'
  ../src/main/cpp/foo.cpp:138: error: undefined reference to '__kmpc_for_static_fini'
  ../src/main/cpp/foo1.cpp:27: error: undefined reference to '__kmpc_fork_call'
  ../src/main/cpp/foo1.cpp:44: error: undefined reference to '__kmpc_fork_call'
  ../src/main/cpp/foo1.cpp:28: error: undefined reference to '__kmpc_global_thread_num'
  ../src/main/cpp/foo1.cpp:28: error: undefined reference to '__kmpc_for_static_init_4'
  ../src/main/cpp/foo1.cpp:27: error: undefined reference to '__kmpc_for_static_fini'
  ../src/main/cpp/foo1.cpp:45: error: undefined reference to '__kmpc_global_thread_num'
  ../src/main/cpp/foo1.cpp:45: error: undefined reference to '__kmpc_for_static_init_4'
  ../src/main/cpp/foo1.cpp:44: error: undefined reference to '__kmpc_for_static_fini'
  ../src/main/cpp/foo1.cpp:76: error: undefined reference to '__kmpc_fork_call'
  ../src/main/cpp/foo1.cpp:77: error: undefined reference to '__kmpc_global_thread_num'
  ../src/main/cpp/foo1.cpp:77: error: undefined reference to '__kmpc_for_static_init_4'
  ../src/main/cpp/foo1.cpp:76: error: undefined reference to '__kmpc_for_static_fini'
  ../src/main/cpp/foo1.cpp:100: error: undefined reference to '__kmpc_global_thread_num'

  clang++: error: linker command failed with exit code 1 (use -v to see invocation)
  ninja: build stopped: subcommand failed.

I just use the following entry in CMakeLists.txt

target_link_libraries (foo_neon -static-openmp)

Am I missing something?

@rpattabi
Copy link

rpattabi commented Apr 1, 2020

I found that static linking requires -fopenmp flag also. It was to be mentioned in compile options as well.

target_link_libraries (foo_neon -static-openmp -fopenmp)

That solves static linking.

Suggestion

A sample project using openmp that is kept up-to-date would help (CMake and ndk-build) to save us some time working these out. Thank you.

@DanAlbert
Copy link
Member

Our tests can't be out of date or they'll fail, so that's always a good place to look for examples:

https://android.googlesource.com/platform/ndk/+/refs/heads/master/tests/device/openmp/
https://android.googlesource.com/platform/ndk/+/refs/heads/master/tests/device/openmp_shared/jni

@hhb
Copy link
Collaborator

hhb commented Jan 23, 2021

CMake upstream is proposing to default to -static-openmp. That sounds reasonable because it is the most common case. Please comment there if you have a different opinion.

@mansourmoufid
Copy link

Hello, there used to be a libomp.so in the sysroot (sdk/ndk/.../toolchains/llvm/prebuilt/darwin-x86_64/sysroot). I can't remember the SDK version exactly, but it was just a few months ago and had this checksum:

% shasum app/libs/arm64-v8a/libomp.so
55129b6fca60342e36dd483322e499567273e9ff  app/libs/arm64-v8a/libomp.so

This week I upgraded Android Studio, installed the new SDK, now libomp.so is here:

sdk/ndk/23.1.7779620/toolchains/llvm/prebuilt/darwin-x86_64/lib64/clang/12.0.8/lib/linux/aarch64/libomp.so

I had a script that read ELF headers of libraries I built outside of Android Studio, it copied all the necessary libraries from the sysroot into app/libs/arm64-v8a/, now I have to do this manually. Please copy libomp.so into the sysroot again.

@DanAlbert
Copy link
Member

DanAlbert commented Feb 15, 2022

That's where the toolchain looks for openmp. The prior location required extra work from every build system just to find the libraries.

@mansourmoufid
Copy link

My mistake, I just realized libomp wasn't in the list of stable native libraries to begin with. I just copy libomp.so manually now, that works for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants