-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Adjust so that we only require FEAT_LSE for osx-arm64 and maccatalyst, not for iOS or tvOS #119032
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
…, not for iOS or tvOS
1fb6bef
to
11bc3a3
Compare
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
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.
If I understand this correctly, we still require AdvSIMD on every arm64 target (which makes sense, iirc it's part of the baseline), but we're changing things so LSE is only required on OSX and MacCatalyst and it's allowed to be missing on iOS and tvOS.
@kg, correct. If we have the minimum iOS/tvOS hardware supported then we can likely be a bit more explicit about them as well. But keeping it at the same as .NET 10 will resolve the CI failure for now (I expect iOS is higher than armv8.0-a in particular) |
// * tvOSSimulator | ||
// | ||
// For osx-arm64 and maccatalyst, we know that the baseline is apple-m1 | ||
// For iOS, tvOS, and the simulator variants it can be older |
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.
nit: technically for the simulator variants this doesn't apply since they can only run on an m1 mac host.
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.
👍, more than happy to update things if we know certain ones support higher.
I expect iOS in practice will also be higher due to the minimum iPhone version supported.
If we have a more explicit set of docs around what the minimums are, I can work on updating and ensuring its properly tested for those targets
@tannergooding should we backport this? |
@akoeplinger this was a .NET 11 only change, so there shouldn't be any need to backport. |
This resolves #119008