diff --git a/build/cmake/android-legacy.toolchain.cmake b/build/cmake/android-legacy.toolchain.cmake index aac77505..b2fdd105 100644 --- a/build/cmake/android-legacy.toolchain.cmake +++ b/build/cmake/android-legacy.toolchain.cmake @@ -456,12 +456,6 @@ list(APPEND ANDROID_LINKER_FLAGS -Wl,--fatal-warnings) list(APPEND ANDROID_LINKER_FLAGS_EXE -Wl,--gc-sections) # Debug and release flags. -list(APPEND ANDROID_COMPILER_FLAGS_DEBUG -O0) -if(ANDROID_ABI MATCHES "^armeabi" AND ANDROID_ARM_MODE STREQUAL thumb) - list(APPEND ANDROID_COMPILER_FLAGS_RELEASE -Oz) -else() - list(APPEND ANDROID_COMPILER_FLAGS_RELEASE -O2) -endif() list(APPEND ANDROID_COMPILER_FLAGS_RELEASE -DNDEBUG) if(ANDROID_TOOLCHAIN STREQUAL clang) list(APPEND ANDROID_COMPILER_FLAGS_DEBUG -fno-limit-debug-info) diff --git a/build/cmake/android.toolchain.cmake b/build/cmake/android.toolchain.cmake index 9a1dae2f..b01340eb 100644 --- a/build/cmake/android.toolchain.cmake +++ b/build/cmake/android.toolchain.cmake @@ -253,11 +253,6 @@ if(DEFINED ANDROID_SO_UNDEFINED AND NOT DEFINED ANDROID_ALLOW_UNDEFINED_SYMBOLS) set(ANDROID_ALLOW_UNDEFINED_SYMBOLS "${ANDROID_SO_UNDEFINED}") endif() -# https://github.com/android/ndk/issues/133 -if(CMAKE_ANDROID_ARCH_ABI MATCHES "^armeabi" AND NOT CMAKE_ANDROID_ARM_MODE) - string(APPEND _ANDROID_NDK_INIT_CFLAGS_RELEASE " -Oz") -endif() - # Exports compatible variables defined in exports.cmake. set(_ANDROID_EXPORT_COMPATIBILITY_VARIABLES TRUE) diff --git a/docs/changelogs/Changelog-r23.md b/docs/changelogs/Changelog-r23.md index 112b77dc..266dfdea 100644 --- a/docs/changelogs/Changelog-r23.md +++ b/docs/changelogs/Changelog-r23.md @@ -32,6 +32,13 @@ For Android Studio issues, follow the docs on the [Android Studio site]. * Update LLVM to clang-r416183c, based on LLVM 12 development. * [Issue 1544]: Now uses universal binaries for M1 Macs. +* [Issue 1536]: Make optimization flags used with CMake more consistent. + Historically thumb release builds used `-Oz`, but AGP switched to using + `RelWithDebInfo` for release builds in the latest release which was not using + `-Oz`. To reduce per-arch differences and behavior differences compared to + CMake's defaults, `-Oz` use was removed. You may see code size increases for + armeabi-v7a due to this, but also increased optimization. To restore the prior + behavior, add `-Oz` to your cflags. * [Issue 1553]: Updated sysroot to latest Android 12. * [Issue 1560]: Fixed pull-up of unsupported API levels when using the new CMake toolchain file. This affects CMake 3.21 and @@ -41,6 +48,7 @@ For Android Studio issues, follow the docs on the [Android Studio site]. during CMake try-compile. * [Issue 1569]: Fixed `-fno-integrated-as` not being able to find the assembler. +[Issue 1536]: https://github.com/android/ndk/issues/1536 [Issue 1544]: https://github.com/android/ndk/issues/1544 [Issue 1553]: https://github.com/android/ndk/issues/1553 [Issue 1560]: https://github.com/android/ndk/issues/1560 diff --git a/ndk/testing/flag_verifier.py b/ndk/testing/flag_verifier.py index 5425dd97..82ed7e37 100644 --- a/ndk/testing/flag_verifier.py +++ b/ndk/testing/flag_verifier.py @@ -161,12 +161,17 @@ def verify_ndk_build(self) -> FlagVerifierResult: f'APP_PLATFORM=android-{self.api}', ]) - def verify_cmake(self) -> FlagVerifierResult: + def verify_cmake( + self, + cmake_flags: Optional[list[str]] = None) -> FlagVerifierResult: """Verifies that CMake behaves as specified. Returns: A FlagVerifierResult object describing the verification result. """ + if cmake_flags is None: + cmake_flags = [] + host = Host.current() if host == Host.Windows64: tag = 'windows-x86' @@ -195,7 +200,7 @@ def verify_cmake(self) -> FlagVerifierResult: f'-DANDROID_USE_LEGACY_TOOLCHAIN_FILE={self.toolchain_mode}', '-GNinja', f'-DCMAKE_MAKE_PROGRAM={ninja}', - ] + ] + cmake_flags result = subprocess.run(cmd, check=False, stdout=subprocess.PIPE, diff --git a/tests/build/cmake_default_flags/__init__.py b/tests/build/cmake_default_flags/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/build/cmake_default_flags/project/CMakeLists.txt b/tests/build/cmake_default_flags/project/CMakeLists.txt new file mode 100644 index 00000000..4cf45623 --- /dev/null +++ b/tests/build/cmake_default_flags/project/CMakeLists.txt @@ -0,0 +1,4 @@ +cmake_minimum_required(VERSION 3.6) +project(CMakeDefaultFlagsTest CXX) + +add_library(foo SHARED foo.cpp) diff --git a/tests/build/cmake_default_flags/project/foo.cpp b/tests/build/cmake_default_flags/project/foo.cpp new file mode 100644 index 00000000..e69de29b diff --git a/tests/build/cmake_default_flags/test.py b/tests/build/cmake_default_flags/test.py new file mode 100644 index 00000000..a7ead75a --- /dev/null +++ b/tests/build/cmake_default_flags/test.py @@ -0,0 +1,34 @@ +# +# Copyright (C) 2021 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +"""Check that the CMake toolchain uses the correct default flags.""" +from pathlib import Path +from typing import Optional + +from ndk.test.spec import BuildConfiguration +from ndk.testing.flag_verifier import FlagVerifier + + +def run_test(ndk_path: str, + config: BuildConfiguration) -> tuple[bool, Optional[str]]: + """Check that the CMake toolchain uses the correct default flags. + + Currently this only tests the optimization flags for RelWithDebInfo, but + it's probably worth expanding in the future. + """ + verifier = FlagVerifier(Path('project'), Path(ndk_path), config) + verifier.expect_flag('-O2') + return verifier.verify_cmake(['-DCMAKE_BUILD_TYPE=RelWithDebInfo' + ]).make_test_result_tuple()