google / gemmlowp

Low-precision matrix multiplication
Apache License 2.0
1.77k stars 451 forks source link

Fix build error on Android #212

Closed ysohma closed 10 months ago

ysohma commented 10 months ago

Hi there,

I'm encountering a build error on the Android platform.

In contrib/CMakeLists.txt, the -lpthread linker flag is added for non-Windows platforms. However, on certain platforms like Android, the -lpthread flag is unnecessary and is causing build errors.

This is simple script to reproduce.

mkdir _build
cd _build
cmake -DCMAKE_TOOLCHAIN_FILE=<NDK-path>/build/cmake/android.toolchain.cmake -DANDROID_ABI=arm64-v8a ../contrib
cmake --build .

# [  5%] Building CXX object 
# CMakeFiles/eight_bit_int_gemm.dir/Users/yoshio.soma/gemmlowp/eight_bit_int_gemm/eight_bit_int_gemm.cc.o
# [ 11%] Linking CXX static library libeight_bit_int_gemm.a
# [ 11%] Built target eight_bit_int_gemm
# [ 17%] Building CXX object CMakeFiles/benchmark.dir/Users/yoshio.soma/gemmlowp/test/benchmark.cc.o
# [ 23%] Linking CXX executable benchmark
# ld: error: unable to find library -lpthread
# clang++: error: linker command failed with exit code 1 (use -v to see invocation)
# make[2]: *** [benchmark] Error 1
# make[1]: *** [CMakeFiles/benchmark.dir/all] Error 2
# make: *** [all] Error 2

Changes

Instead of hard-coding the -lpthread flag, I propose using find_package() to link with pthread. By employing find_package(), CMake will automatically detect the platform and set the appropriate flags accordingly.

Related Issue

I believe this change will resolve the build error on the Android platform. Your feedback and review are much appreciated.

Thank you!

wowo68 commented 10 months ago

这是来自QQ邮箱的假期自动回复邮件。你好,我最近正在休假中,无法亲自回复你的邮件。我将在假期结束后,尽快给你回复。

bjacob commented 10 months ago

Thanks for the PR. It seems reasonable. I don't seem to have permissions anymore to merge PRs here, as I am no longer a Google employee. @mariecwhite can you take a look? No need to import this into the Google internal code repository (a manual process anyway, no copybara here). This should be GitHub-only - just merge this PR here.

mariecwhite commented 10 months ago

Looks like I don't have permissions to merge here. Let me see if I can get write access.

mariecwhite commented 10 months ago

I now have write access. PR merged.