-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[test] Fix or disable tests for 32-bit platforms #82501
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that the target triple is armv7-unknown-linux-androideabi, while the Swift compiler passes along the slightly different module triple armv7-unknown-linux-android to the ClangImporter. I think some other platforms have similar differences, so I don't know why this would be failing on Android armv7 alone, unless it really is the only one on the official CI that varies like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fix failed on macOS, as it passes the full x86_64-apple-macosx13.0 triple to the ClangImporter, not the abbreviated module triple x86_64-apple-macos like Android armv7 does. Since these platforms are doing different things, simply disabled this test for Android armv7 for now.
test/IRGen/abitypes_arm.swift
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Android armv7 generated define internal void @makeOne(ptr dead_on_unwind noalias writable sret(%struct.One) align 4 %agg.result, float noundef %f, float noundef %s) in its IR instead, so I added those attributes, optionally since I don't know if they will be there on other armv7 platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like Clang adds dead_on_unwind for sret args, so they always appear in a combination. I guess we don't need to make it optional. https://github.com/swiftlang/llvm-project/blob/f1607d165cb61ef9f857d76afc6e8a04345bb41a/clang/lib/CodeGen/CGCall.cpp#L2622
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, do you mind approving this pull as is? Don't want to kick off another CI run just for this, and keeping it optional is more flexible anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
llvm-cxxfilt _ZnwjPv15container_new_t demangles this to operator new(unsigned int, void*, container_new_t), in addition to the operator new(unsigned long, void*, container_new_t) that was already there for 64-bit platforms.
|
@swift-ci smoke test |
|
@swift-ci smoke test macos |
Fix two IRGen tests that are failing on Android armv7 and disable eight ClangImporter, C++ Interop, and SILOptimizer tests, two of which that were already failing on other 32-bit platforms.
|
Simply disabled the five failing C++ Interop tests and the ClangImporter test for Android armv7 and rebased. |
|
@swift-ci smoke test |
|
@swift-ci smoke test macos |
|
@kateinoigakukun, mind reviewing the two IRGen changes? The rest are just disabling tests for Android armv7, so should be fine. |
Fix two IRGen tests that are failing on Android armv7 and disable eight ClangImporter, C++ Interop, and SILOptimizer tests, two of which that were already failing on other 32-bit platforms.
Fix one PCM and two IRGen tests that are failing on Android armv7 and disable two SILOptimizer tests that were already failing on other 32-bit platforms.
As part of the upcoming official Android SDK bundle, #80788, we're running the non-executable compiler validation suite for Android armv7 again before including the armv7 Swift stdlib, and I see 10 tests failing in both the trunk 6.3 and 6.2 branches.
This pull fixes or disables half of them, whereas the other half are all C++ Interop constructor tests that fail because they cannot build that C module with
size_tissues:@egorzhdan or @fahadnayyar, any idea what that 32-bit C++ Interop issue is or should I just disable those five tests for Android armv7?