-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[stdlib][SR-2239]: add support for AArch64 variadics #20708
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
[stdlib][SR-2239]: add support for AArch64 variadics #20708
Conversation
[stdlib][SR-2239]: add support for AArch64 variadics
Like #20709, we don't really take additive changes on release branches, especially when we're already up to working on the next release. Let's put this on master instead, and we can cherry-pick to 5.0. |
@jrose-apple are you refusing to merge this PR? |
@bob-wilson already explained our release process. The final authority would be the 4.2 release manager, @tkremenek. |
Aside: we especially aren't going to take any changes that haven't been integrated into master yet. |
This is simply not true. There are numerous examples of changes to release branches that are not integrated in the master. These changes only affect AArch64 on Linux and have no impact on any other systems. The purpose is to enable successful builds of 4.2 on the Swift Community CI. Achieving this is extremely important for motivation and moral of all involved in bringing Open Source Swift to Arm devices. After looking at 1000's of build:failing for the past few months in would really nice to see build:passing for a change. We can then have an "Official Swift Community-Hosted" release of Swift 4.2 for AArch64 on Linux. Then we can look at integrating these changes and others into the master and moving toward having AArch64 on Linux officially supported in future releases. @tkremenek can we get a decision on this please. |
The problem is, @jrose-apple that master has diverged too far to allow this to be cherry-picked into it. This puts the Swift Arm community in a position again to maintain parallel repos. We've tried to work with the core team about this over the years, and they've expressed interest in helping us stay on mainline. However, not being willing to work with us pushes us away again. Swift 5 has not been released, and it's not usable for any user in production settings, let alone on Arm64. Saying that we should accept the fact that Swift 4.2 doesn't work on Arm64, and never will because we need to wait until some indeterminate point in the future (WWDC 2019?), when Swift 5 is released is pretty painful. If you look at the changes proposed, they are incredibly minor. In fact, all but one change is gated in |
I'm not in control of the policy here; I'm just reporting what we've done in the past. But we don't take changes on release branches precisely because master changes enough that things get out of date between releases. If the change goes in here, and you never get it into the 5.0 branch, then Swift 4.2.x will work with AArch64 but Swift 5.0 won't. That's not a state the official project releases should be in. |
Can we start with the question of getting this into master? We really do want the Swift 4.2 branch to be frozen at this point, but perhaps we could consider an exception. Ted would need to approve that. But, regardless, it is a very important part of the development process for Swift that all work should first be done on the master branch. If the code has diverged too far for a simple cherry-pick to work, then you may have to reimplement the changes. You should get the community-hosted CI working for both the master and Swift 5.0 branches before we consider pulling any changes back to the 4.2 branch. |
It's worth pointing out that the velocity of work in mainline swift is such that we're always catching up. There's a chicken-and-egg issue where the lack of community CI means that we deal with regressions introduced between the months it takes our "team" of 1-3 volunteers to get through struggles. Morale in this team is so low that we may be down to 1 member before too long, as this PR push is one of those member's last attempt to move forward. We face the very real possibility that Swift on arm will no longer be supported on anything beyond Swift 4 by this fact alone. |
@bob-wilson swift-5.0-branch has now been added - https://ci-external.swift.org/view/swift-5.0-branch/job/oss-swift-5.0-RA-linux-ubuntu-16.04-aarch64/ |
Thanks! That's great. I'm sympathetic to the challenge of keeping up with the velocity of changes in Swift. As long as your changes don't affect other platforms, we should try to be as flexible as possible in getting your changes merged. Can you create a PR to get this change (or the equivalent) on the master branch? |
@bob-wilson I am working on getting this done in the near future. Unfortunately the swift-5.0-branch and the master no longer build on AArch64. I don't like to submit a PR without first achieving successful builds and testing first. By the time we fix all the new issues with 5.0 we will probably be in the same situation as we are now in with trying to get changes added to 4.2. |
There will likely be more 4.2 releases for Linux, so I'm open to taking this PR. However, the platform support should be in |
@tkremenek Thanks for looking at this. |
@swift-ci clean test |
@tkremenek @bob-wilson As it turns out the numerous issues we were having trying get As a result we have been able to move forward a lot sooner than I was expecting. PR's for AArch64 variadics support have now been uploaded for |
Fantastic work! Thanks so much! |
@futurejones this is great. We'll get these reviewed and merged as soon as possible into the respective branches. We all appreciate the efforts that have gone into making this port happen! One thing that came out of this discussion is that we should have clearer documentation guidance for folks working on new platform support on how that work should proceed. Specifically:
I know it is tempting to first work on a previous release branch, but that doesn't match with the development model of the project and can lead to work not continuing to live on going forward. |
FWIW, I've had pretty good luck with keeping swift master working on android aarch64. That really is close enough to Linux aarch64 that there isn't much broken I suspect. Now, I don't know if that is due to my local patchset or not, so, having a set of SRs on JIRA to track the issues that you are seeing would be useful. One "undocumented" trick that I have though is that I am using lld and not bfd (which is unusable) nor gold (which does have a few issues). The only thing that I hadn't actually gotten around to fixing properly was the variadics which @tkremenek did push into master (with follow up comments from @rjmccall and myself). |
@rjmccall any concerns with landing this change as is for 4.2.3? |
No, this looks great; sorry for being slow to respond. |
@tkremenek @rjmccall Thank you! |
Adds support for AArch64 variadics.
This PR along with #19462 will allow successful builds of
swift-4.2-branch
on Linux AArch64 Ubuntu 16.04.This is an updated version of #15174
Resolves SR-2239.
@jrose-apple @compnerd can you push #19462 to the
swift-4.2-branch
With these changes we can finally see successful builds on https://ci-external.swift.org/job/oss-swift-RA-linux-ubuntu-16.04-aarch64/
Then we should be able to have an official release of Swift 4.2 for Ubuntu 16.04 AArch64.