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

Android: Remove -fno-integrated-as, it can break arm64v8 build #48851

Merged

Conversation

akien-mga
Copy link
Member

We found that this flag causes this error on PR #48812 which does not add any
fancy inline assembly:

/tmp/tile_set-ce236a.s: Assembler messages:
/tmp/tile_set-ce236a.s:34676: Error: selected processor does not support `bfc x0,#32,#32'
clang++: error: assembler command failed with exit code 1 (use -v to see invocation)

That flag is mentioned in various errors related to assembler failures on
arm64v8 with Clang from the Android NDK.

It was added in Godot in #6958 when migrating from GCC to Clang, and is indeed
referenced in the NDK's Clang migration guide:
https://android.googlesource.com/platform/ndk/+/master/docs/ClangMigration.md

Especially for ARM and ARM64, Clang is much stricter about assembler rules
than GCC/GAS. Use -fno-integrated-as if Clang reports errors in inline
assembly or assembly files that you don't wish to modernize.

We don't get those errors nowadays so it seems the flag is no longer needed.


I'll do a test round for master and 3.3 on all Android architectures to be safe.

We found that this flag causes this error on PR godotengine#48812 which does not add any
fancy inline assembly:
```
/tmp/tile_set-ce236a.s: Assembler messages:
/tmp/tile_set-ce236a.s:34676: Error: selected processor does not support `bfc x0,godotengine#32,godotengine#32'
clang++: error: assembler command failed with exit code 1 (use -v to see invocation)
```

That flag is mentioned in various errors related to assembler failures on
arm64v8 with Clang from the Android NDK.

It was added in Godot in godotengine#6958 when migrating from GCC to Clang, and is indeed
referenced in the NDK's Clang migration guide:
https://android.googlesource.com/platform/ndk/+/master/docs/ClangMigration.md

> Especially for ARM and ARM64, Clang is much stricter about assembler rules
> than GCC/GAS. Use `-fno-integrated-as` if Clang reports errors in inline
> assembly or assembly files that you don't wish to modernize.

We don't get those errors nowadays so it seems the flag is no longer needed.
@akien-mga akien-mga added bug platform:android topic:buildsystem cherrypick:3.x Considered for cherry-picking into a future 3.x release cherrypick:3.3 labels May 19, 2021
@akien-mga akien-mga added this to the 4.0 milestone May 19, 2021
@akien-mga akien-mga requested a review from a team as a code owner May 19, 2021 17:02
Copy link
Member

@groud groud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixed the issue on my PR (I checked the change on it)

@akien-mga
Copy link
Member Author

I'll do a test round for master and 3.3 on all Android architectures to be safe.

Both tests built fine with NDK 21.4 (built Mono flavor for 3.3 and classical for master).

@akien-mga akien-mga merged commit 9004943 into godotengine:master May 19, 2021
@akien-mga akien-mga deleted the android-remove-fno-integrated-as branch May 19, 2021 17:42
@akien-mga
Copy link
Member Author

Cherry-picked for 3.4.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label May 19, 2021
@akien-mga
Copy link
Member Author

Cherry-picked for 3.3.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants