Skip to content

[android] Fix IRGen/condfail.sil test for Android ARMv7/AArch64. #23580

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

Merged
merged 1 commit into from
Mar 28, 2019

Conversation

drodriguez
Copy link
Contributor

For AArch64, the fix is duplicating the arm64 lines, since those are not
taken when testing Android/Linux which use AArch64 as the architecture.

For ARMv7, the change is supporting both trap and .inst 0xe7ffdefe.
According to llvm/lib/Target/ARM/ARMAsmPrinter.cpp, non-Darwin binutils
do not support the mnemonic trap, so an .inst is emitted instead. The
same instruction has to be used in both places, though.

For both architectures add the Android version of APP/NO_APP, which uses
@-symbols instead of ##.

@drodriguez drodriguez requested a review from compnerd March 26, 2019 23:02
For AArch64, the fix is duplicating the arm64 lines, since those are not
taken when testing Android/Linux which use AArch64 as the architecture.

For ARMv7, the change is supporting both trap and .inst 0xe7ffdefe.
According to llvm/lib/Target/ARM/ARMAsmPrinter.cpp, non-Darwin binutils
do not support the mnemonic trap, so an .inst is emitted instead. The
same instruction has to be used in both places, though.

For both architectures add the Android version of APP/NO_APP, which uses
@-symbols or // instead of ##.
@drodriguez drodriguez force-pushed the android-condfail-test branch from a7f753d to 8c6bda7 Compare March 27, 2019 17:52
@drodriguez
Copy link
Contributor Author

Modified to have different APP/NO_APP checks for Android ARMv7 and AArch64. This fix that the checks were not actually checked at all (I haven’t fixed the Windows ones, but I’m quite sure that they are not triggering, btw).

Also remove the variable for the trap instruction. It doesn’t give us a lot more safety, and x86_64 does something similar with .cfi_startproc|Lfunc_begin

@compnerd
Copy link
Member

@swift-ci please test and merge

@swift-ci swift-ci merged commit ecb539f into swiftlang:master Mar 28, 2019
@drodriguez drodriguez deleted the android-condfail-test branch April 8, 2019 20:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants