Merge "Admit that we lost -Oz in CMake." into snap-temp-L328000009512…

…75450

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)

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)
 

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

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,

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)

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()