Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

r26 armeabi-v7a libc++ probably not built as thumb #1953

Closed
znakeeye opened this issue Oct 19, 2023 · 10 comments
Closed

r26 armeabi-v7a libc++ probably not built as thumb #1953

znakeeye opened this issue Oct 19, 2023 · 10 comments
Labels

Comments

@znakeeye
Copy link

znakeeye commented Oct 19, 2023

Description

Merely switching from 25.1.8937393 to 26.1.10909125 causes my .so files to decrease somewhat in size, except for arm64-v8a. Is this expected?

Also, the libc++_shared.so is larger for all ABIs. Also expected?

| Library          | ABI         | r25 (bytes)  | r26 (bytes)  |
| ---------------- | ----------- |-------------:|-------------:|
| mylib.so         | arm64-v8a   |    2 395 376 |    2 465 384 | <---
| mylib.so         | armeabi-v7a |    2 231 984 |    2 229 208 |
| mylib.so         | x86         |    2 755 532 |    2 718 636 |
| mylib.so         | x86_64      |    2 727 504 |    2 679 816 |
| libc++_shared.so | arm64-v8a   |    1 026 616 |    1 330 832 | ?
| libc++_shared.so | armeabi-v7a |      610 272 |    1 109 732 | ?
| libc++_shared.so | x86         |      993 108 |    1 284 616 | ?
| libc++_shared.so | x86_64      |    1 045 960 |    1 275 840 | ?

I did set minSdk 21 in build.gradle, but my Application.mk (accidently) contained APP_PLATFORM := android-19, so I'm not entirely sure which SDK was configured for the NDK build. It should be noted that mylib.so is built in a project which depends on an .AAR (where APP_STRIP_MODE was set to none). Otherwise, no fancy settings.

Application.mk

APP_PLATFORM := android-21
APP_ABI := armeabi-v7a x86 arm64-v8a x86_64
APP_STL := c++_shared

Attaching the .so files for arm64-v8a:
arm64-libs.zip

Affected versions

r26

Canary version

No response

Host OS

Windows

Host OS version

Windows 11

Affected ABIs

armeabi-v7a, arm64-v8a, x86, x86_64

Build system

ndk-build

Other build system

No response

minSdkVersion

21

Device API level

No response

@znakeeye znakeeye added the bug label Oct 19, 2023
@github-project-automation github-project-automation bot moved this to Triaged in NDK r26b Oct 23, 2023
@DanAlbert DanAlbert removed this from NDK r26b Oct 23, 2023
@github-project-automation github-project-automation bot moved this to Triaged in NDK r26c Oct 23, 2023
@DanAlbert
Copy link
Member

Tentatively triaging to r26c. These are often quite difficult to diagnose when they weren't the result of something we did on purpose, and they're generally caused by upstream LLVM changes, and that can be anything from a change in optimization behavior to a correctness fix.

@pirama-arumuga-nainar could one of you have a look and see if you can come up with an explanation?

@appujee
Copy link
Collaborator

appujee commented Oct 24, 2023

the libc++_shared.so in r26 has more symbols.

only_in_r25.txt
only_in_r26.txt

@DanAlbert
Copy link
Member

Is that the reason for mylib.so growing though?

| libc++_shared.so | armeabi-v7a | 610 272 | 1 109 732 | ?

This is drastic enough that I'd bet the LLVM build doesn't build the runtimes as thumb... @rprichard

(the other architectures grew, but considering we were quite behind on libc++, it's not surprising that they've added more code to libc++ in that time)

@appujee
Copy link
Collaborator

appujee commented Oct 24, 2023

For the specific binary liborxDemo.so shared in the bug report. the biggest contributors are .text and .ehframe

section r25 r26
.eh_frame 171836 207984
.text 1372480 1398416

@appujee
Copy link
Collaborator

appujee commented Oct 24, 2023

He is a detailed breakdown of the symbols if anyone is interested in doing further analysis. It will take a while to find specific symbols but if the bug author is super interested, these files have sufficient information.

r25.csv
r26.csv

@DanAlbert DanAlbert changed the title r26 produces larger .so file for arm64-v8a (and libc++_shared.so larger for all ABIs) r26 armeabi-v7a libc++ probably not built as thumb Nov 1, 2023
@DanAlbert
Copy link
Member

The toolchain folks say that the size increase for arm64 is within the expected range (new compiler means new behavior, different optimization decisions, and this size difference is within what we expect for that).

The arm32 libc++ size increase does look like it wasn't built as thumb though (libc++ moved from the NDK to being part of the toolchain in r26, so the build process was redone). That's a thing we ought to fix.

That alone won't be the trigger for r26c, but we'll pick up that fix in r26c when something else inevitably does.

@pirama-arumuga-nainar
Copy link
Collaborator

With https://android-review.googlesource.com/c/toolchain/llvm_android/+/2814878, the size of the arm libc++.so is 888K - it's still larger than r25 but it is expected with new compiler and libc++ (and in line with the rest of the platforms.)

@znakeeye
Copy link
Author

znakeeye commented Dec 1, 2023

Is that the reason for mylib.so growing though?

| libc++_shared.so | armeabi-v7a | 610 272 | 1 109 732 | ?

This is drastic enough that I'd bet the LLVM build doesn't build the runtimes as thumb... @rprichard

(the other architectures grew, but considering we were quite behind on libc++, it's not surprising that they've added more code to libc++ in that time)

You know this better than me. But in our AAR project, we do set LOCAL_ARM_MODE := arm when building the library from prefabs. When the AAR is consumed by the app project, we do the same. Maybe this is related?

@DanAlbert
Copy link
Member

Your flags have no impact on the already-built libc++_shared.so.

@DanAlbert DanAlbert moved this from Triaged to Merged in NDK r26c Feb 1, 2024
@github-project-automation github-project-automation bot moved this from Merged to Triaged in NDK r26c Feb 1, 2024
@DanAlbert DanAlbert moved this from Triaged to Merged in NDK r26c Feb 1, 2024
@DanAlbert DanAlbert added this to NDK r27 Feb 1, 2024
@github-project-automation github-project-automation bot moved this to Unconfirmed in NDK r27 Feb 1, 2024
@DanAlbert DanAlbert moved this from Unconfirmed to Needs prebuilt update in NDK r27 Feb 1, 2024
@DanAlbert
Copy link
Member

Fixed in r26, but we need an update for r27 still.

@DanAlbert DanAlbert reopened this Feb 1, 2024
@github-project-automation github-project-automation bot moved this from Merged to Triaged in NDK r26c Feb 1, 2024
@github-project-automation github-project-automation bot moved this from Needs prebuilt update to Triaged in NDK r27 Feb 1, 2024
@DanAlbert DanAlbert moved this from Triaged to Merged in NDK r26c Feb 1, 2024
@DanAlbert DanAlbert moved this from Triaged to Needs prebuilt update in NDK r27 Feb 1, 2024
Hemalv02 pushed a commit to Hemalv02/llvm_android that referenced this issue Feb 3, 2024
Bug: android/ndk#1953

Change-Id: I6d51c995450e98490a70d0ac745bb46be601b102
Test: build.py and check size of arm32 libc++
(cherry picked from commit 44302de)
grendello added a commit to dotnet/android that referenced this issue Feb 19, 2024
Changes: https://github.com/android/ndk/wiki/Changelog-r26#r26c

  * Updated LLVM to clang-r487747e. See AndroidVersion.txt and clang_source_info.md 
     in the toolchain directory for version information.
  * [Issue 1928](android/ndk#1928): Fixed Clang crash in 
     instruction selection for 32-bit armv8 floating point.
  * [Issue 1953](android/ndk#1953): armeabi-v7a libc++ 
     libraries are once again built as thumb.

`AndroidVersion.txt` contains:

```
17.0.2
based on r487747e
```
@github-project-automation github-project-automation bot moved this from Needs prebuilt update to Merged in NDK r27 Mar 12, 2024
bvlgah pushed a commit to bvlgah/llvm_android_mirror that referenced this issue Mar 28, 2024
Bug: android/ndk#1953

Change-Id: I6d51c995450e98490a70d0ac745bb46be601b102
Test: build.py and check size of arm32 libc++
@github-project-automation github-project-automation bot moved this to Prebuilts submitted in LLVM Aug 29, 2024
foxsen pushed a commit to android-la64/platform-ndk that referenced this issue Oct 10, 2024
Bug: android/ndk#1928
Bug: android/ndk#1953
Test: None
Change-Id: Iedf3172f36d66df94565e8cb4dbdf704f512304b
foxsen pushed a commit to android-la64/platform-ndk that referenced this issue Oct 10, 2024
Bug: android/ndk#1728
Bug: android/ndk#1853
Bug: android/ndk#1928
Bug: android/ndk#1947
Bug: android/ndk#1953
Bug: android/ndk#1963
Bug: android/ndk#1973
Bug: android/ndk#1988
Test: ./checkbuild.py && ./run_tests.py
Change-Id: Ib319c4f80c107e57ede6b6130d81d6d1cd87e22d
jaavid pushed a commit to J-Androids/superproject that referenced this issue Feb 10, 2025
Bug: android/ndk#1953

Change-Id: I6d51c995450e98490a70d0ac745bb46be601b102
Test: build.py and check size of arm32 libc++
(cherry picked from commit 44302de9e2f9c8655542a003c3631e4360fb8457)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Prebuilts submitted
Status: Merged
Status: Merged
Development

No branches or pull requests

4 participants