Skip to content

CoreFoundation: add a workaround for Linux with the rebranch #3086

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
Oct 11, 2021

Conversation

compnerd
Copy link
Member

This adds an optimization barrier in the loop that is potentially
getting mis-optimized with the rebranch on Linux. This should allow us
to rebranch and follow up with a proper fix subsequently.

Thanks to @etcwilde for tracking down this issue!

@compnerd
Copy link
Member Author

@swift-ci please test

Avoid optimizations on Linux as LICM, LoadStoreHoisting, and inlining
seems to play bad tricks on range merging.  This allows the
AttributedString tests to pass on Linux after the rebranch.

SR-15302 tracks resolving the underlying issue(s).

Thanks to @etcwilde for tracking down this issue!
@compnerd
Copy link
Member Author

@swift-ci please test

@etcwilde
Copy link
Contributor

Do we want to leave a note that we know that there is an issue with leftEnd being optimized wrong?

@compnerd
Copy link
Member Author

I think that's better on the JIRA issue; I suspect that once we get a reduced test case, we can talk about this more reliably. It feels like we don't have a strong enough grasp as we couldn't localize it to a single function, and trying to isolate a set of functions seems ... like time better spent on fixing the issue. I would however agree that the change should have a comment referencing the SR .... which I swear I put in. :sigh: I don't want to touch this until the results are back from the associated PR. Worst case, a follow up to add that is something I might argue for, just to avoid the CI penalty 👎

@etcwilde
Copy link
Contributor

I would however agree that the change should have a comment referencing the SR .... which I swear I put in. :sigh: I don't want to touch this until the results are back from the associated PR. Worst case, a follow up to add that is something I might argue for, just to avoid the CI penalty

That's fine by me.

@shahmishal shahmishal merged commit 0951fc8 into swiftlang:main Oct 11, 2021
@compnerd compnerd deleted the barrier branch October 11, 2021 16:01
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