cmake: Add fix compilation for Android armeabi-v7a #5702
Conversation
|
Fixes issue as described in: |
| if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Android") | ||
| # Android armeabi-v7a | ||
| list(APPEND ARCH_FLAGS -mfpu=neon-vfpv4 -mno-unaligned-access -funsafe-math-optimizations) | ||
| else() | ||
| # Raspberry Pi 2 | ||
| list(APPEND ARCH_FLAGS -mfpu=neon-fp-armv8 -mno-unaligned-access -funsafe-math-optimizations) | ||
| endif() |
There was a problem hiding this comment.
Is there a reason we are explicitly specifying -mfpu anyway? Shouldn't the default of -march=native cause the compiler to use the appropriate -mfpu by default?
There was a problem hiding this comment.
Should, I have no idea why the auto setting is overriden here as well.
I've crosscompiled for the Android armeabi-v7a with removed -mfpu flag and it compiled successfully as well. I've only set the flag to neon-vfpv4 as suggested in the attached Issue (#4421). Maybe @prusnak could shed some light onto the meaning of those flags for arm v6 and v7?
There was a problem hiding this comment.
auto only works if you are compiling on the target system
specifying flags explicitly allows you to crosscompile for different system
There was a problem hiding this comment.
Based on this we should be able to replace -mfpu with -march and the compiler would do the rest for us. I do not own any of these Raspberries so I cannot even test this myself unfortunately.
There was a problem hiding this comment.
I don't think it should be the build script's job to guess what kind of FPU you have based only on CMAKE_SYSTEM_PROCESSOR and CMAKE_SYSTEM_NAME anyway - best to explicitly specify the necessary -march and -mcpu (and -mfpu if desired, but shouldn't be necessary) in your toolchain file's CMAKE_C_FLAGS_INIT/CMAKE_CXX_FLAGS_INIT.
There was a problem hiding this comment.
Question is then if we want to completely get rid of setting those flags?
I'm not that familiar with llama.cpp contribution guidelines so I do not know if there's any plan for such thing.
At the moment, I'd say it'd make sense given current build script structure.
There was a problem hiding this comment.
I suppose the problem is that Android builds are often cross-compiled (i.e. you almost never build on the actual device).
Agree it's a bit of a mess with these flags as it is (same in whisper.cpp). But since we lack all the various hardware to test so many cases, we can patch stuff one-by-one
| if ("${CMAKE_SYSTEM_NAME}" STREQUAL "Android") | ||
| # Android armeabi-v7a | ||
| list(APPEND ARCH_FLAGS -mfpu=neon-vfpv4 -mno-unaligned-access -funsafe-math-optimizations) | ||
| else() | ||
| # Raspberry Pi 2 | ||
| list(APPEND ARCH_FLAGS -mfpu=neon-fp-armv8 -mno-unaligned-access -funsafe-math-optimizations) | ||
| endif() |
There was a problem hiding this comment.
I suppose the problem is that Android builds are often cross-compiled (i.e. you almost never build on the actual device).
Agree it's a bit of a mess with these flags as it is (same in whisper.cpp). But since we lack all the various hardware to test so many cases, we can patch stuff one-by-one
No description provided.