[BUG] NDK-r25 CMAKE_BUILD_TYPE=MinSizeRel(-Os) CMAKE_BUILD_TYPE=Release(null), NDK-r22b get MinSizeRel(-Os) Release(-O2)

DaydreamCoding commented 2 years ago

Description

The NDK r25 compile with CMAKE_BUILD_TYPE=Release missing default compiler optimizations options.

R22b

MinSizeRel -Os -DNDEBUG -fPIE
Release -O2 -DNDEBUG -fPIE

R25, R24, R23c

MinSizeRel -Os -DNDEBUG -fPIE
Release -DNDEBUG -fPIE

Example: project https://github.com/DaydreamCoding/neon-intrinsics-test/tree/1c123bc9391eec4f9c8305d17f9d24e69d70f237

R22b CMAKE_BUILD_TYPE=Release export ANDROID_NDK=$ANDROID_NDK_R22b ./build/android_build_Release.sh cat build/libsample/build/android/arm64-v8a/test/CMakeFiles/test.dir/flags.make CXX_FLAGS = -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security -O2 -DNDEBUG -fPIE
R22b CMAKE_BUILD_TYPE=MinSizeRel export ANDROID_NDK=$ANDROID_NDK_R22b ./build/android_build_MinSizeRel.sh cat build/libsample/build/android/arm64-v8a/test/CMakeFiles/test.dir/flags.make CXX_FLAGS = -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security -Os -DNDEBUG -fPIE
R25 CMAKE_BUILD_TYPE=Release export ANDROID_NDK=$ANDROID_NDK_R25 ./build/android_build_Release.sh cat build/libsample/build/android/arm64-v8a/test/CMakeFiles/test.dir/flags.make CXX_FLAGS = -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security -DNDEBUG -fPIE
R25 CMAKE_BUILD_TYPE=MinSizeRel export ANDROID_NDK=$ANDROID_NDK_R25 ./build/android_build_MinSizeRel.sh cat build/libsample/build/android/arm64-v8a/test/CMakeFiles/test.dir/flags.make CXX_FLAGS = -g -DANDROID -fdata-sections -ffunction-sections -funwind-tables -fstack-protector-strong -no-canonical-prefixes -D_FORTIFY_SOURCE=2 -Wformat -Werror=format-security -Os -DNDEBUG -fPIE

This is very confusing behavior, R22b Release and MinSizeRel both have default compilation optimizations, R25 MinSizeRel does and Release does not

cmake --version
cmake version 3.23.2

CMake suite maintained and supported by Kitware (kitware.com/cmake).

Associated issue https://github.com/android/ndk/issues/1693 https://github.com/android/ndk/issues/1607

Affected versions

r25

Canary version

No response

Host OS

Mac

Host OS version

macOS 12.4

Affected ABIs

arm64-v8a

Build system

CMake

Other build system

No response

minSdkVersion

21

Device API level

No response

DanAlbert commented 2 years ago

Here are some relevant discussions for r23b (probably not useful until you've read the rest of this post though):

https://github.com/android/ndk/issues/1536 https://android-review.googlesource.com/c/platform/ndk/+/1826817 https://github.com/android/ndk/wiki/Changelog-r23#r23b

The changelog mentions only -Oz, not -O2, which was a gap in the documentation. If I use your build script with r23b, I see -O3 rather than nothing or -O2. That was intentional. Like the bug says, the bandaid was already off, so we took the opportunity to align with CMake's upstream behavior. CMake uses -O3 for Release, -Os for MinSizeRel, and -O2 for RelWithDebInfo.

This has actually been broken since r23b, but was asymptomatic in the default configuration because it's only present in the legacy toolchain file, and even then only in a CMake configuration that AGP does not use. If I force r23b to use the legacy toolchain, I get the same behavior there. It seems that Release (and only Release) is missing optimization flags. MinSizeRel and RelWithDebInfo are getting -Os and -O2 respectively as expected.

I'll upload a fix shortly, since as best as I can make out this was not intentional. It's not strictly a regression either (the same configuration behaves the same way), but since the default behavior has changed to include this bug, I think this is worth getting into r25b (no current release plans for that, but I'll start kicking those tires since we also have some toolchain fixes we want to ship).

DanAlbert commented 2 years ago

https://android-review.googlesource.com/c/platform/ndk/+/2168845

Thanks for the detailed bug report and repro instructions btw. It still took a lot of head scratching to figure out what broke and when, but it would have been a lot harder without that :)

DanAlbert commented 2 years ago

https://ci.android.com/builds/branches/aosp-master-ndk/grid?head=8881098&tail=8881098 has the fix if you want to double check. I added tests to cover the missing configurations, but I haven't retested your repro case against the fixed NDK.

DaydreamCoding commented 2 years ago

Thanks for your reply and fix. with r25b we can distribute this version on a large scale without having to change a lot of source code build parameters. this will be the most reliable version since the r22b.

android / ndk