ermig1979 / Simd

C++ image processing and machine learning library with using of SIMD: SSE, AVX, AVX-512, AMX for x86/x64, VMX(Altivec) and VSX(Power7) for PowerPC, NEON for ARM.
http://ermig1979.github.io/Simd
MIT License
2.01k stars 407 forks source link

ARMv7 linking issue #263

Closed robinvanemden closed 7 months ago

robinvanemden commented 7 months ago

Recently, we updated SIMD from v5.2 (December 2022) to the latest release (v5.4.132) for one of our applications. We got the following error when linking the static ARMv7 version of the lib during compilation of the application:

sclbl_accelerate.cpp:(.text.startup._GLOBAL__sub_I__Z18importDataToTensorPN11arm_compute6TensorEPf+0x44): undefined reference to `Simd::Neon::GetEnable()'
collect2: error: ld returned 1 exit status

Also, the size of the compiled ARMv7 version of the (static) library has gone from 9.8Mb for v5.2 to 3.3Mb for v5.4. The AARCH64 and X64 versions of the library link and compile perfectly fine.

ermig1979 commented 7 months ago

Hi! The cross platform building of Simd Library for ARM is performed after each commit (see .github/workflows/cmake.yml). And there are no error for last month during this action. I tried to build Simd on ARM manually and it was fine. So I need more info to help you.

P.S. As to reducing of size of Simd Library binaries, I would check SIMD_SYNET cmake parameter. If it is off, the size of library can reduce in several times.

robinvanemden commented 7 months ago

Hi! Thank you for your fast response.

The building process for all architectures we support (ARMv7, AARCH64, X64) works seamlessly. However, issues arise only when I use the library to compile the final static binary. Specifically, the ARMv7 compilation using the compiled library fails, displaying an error: undefined reference to Simd::Neon::GetEnable(). It's important to note that SIMD_SYNET remains enabled, as it was before.

We actually did not change anything in our compilation scripts, we just updated SIMD source code from 5.2 to 5.4.

The issue is exclusive to ARMv7; AARCH64 and X64 architectures operate without any problems. Considering this, I wonder if you might have any insights into why ARMv7 SIMD versions 5.2 and 5.4 exhibit different behaviors, particularly regarding the 'undefined reference to Simd::Neon::GetEnable()' error that is being thrown.

I will see if there is some way to circumvent the issue on our side, and keep you posted!

robinvanemden commented 7 months ago

This issue originated from commit af571ee, which introduced a new condition in the test for Clang:

if( NOT (CMAKE_CXX_COMPILER_ID STREQUAL "Clang") AND (CMAKE_CXX_COMPILER MATCHES "clang"))
  set(CXX_NEON_FLAG "-mfpu=neon -mfpu=neon-fp16")
endif()

This condition incorrectly evaluates to false for any standard GNU compiler, leading to an improper setting of CXX_NEON_FLAG. It appears the intention was to exclude setting CXX_NEON_FLAG when both CMAKE_CXX_COMPILER_ID indicates Clang and CMAKE_CXX_COMPILER matches 'clang'. If so, the following would be correct:

if(NOT (CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND CMAKE_CXX_COMPILER MATCHES "clang"))
  # Set NEON flags only if the above condition is true
  set(CXX_NEON_FLAG "-mfpu=neon -mfpu=neon-fp16")
endif()

Alternatively, just using the original line without condition

set(CXX_NEON_FLAG "-mfpu=neon -mfpu=neon-fp16")

could also be a feasible solution. This approach is simpler and might be safer, assuming that potential warnings for Clang type compilers are acceptable and can be ignored.

I can of course create an MR for either option if that would be useful.

ermig1979 commented 7 months ago

Thank you for so detail bug report. I was able to reproduce the issue. The bug was fixed in last commit. It would be great if you perform independent checking.

robinvanemden commented 7 months ago

We tested extensively, and this has resolved our issue. Thanks again!