Skip to content
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

init: avoid an undesirable compiler optimization #42377

Merged
merged 1 commit into from
Sep 28, 2021
Merged

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Sep 24, 2021

jl_current_task is not constant after this point in the function, so we
split the function so that the compiler won't try to optimize it
incorrectly (hoisting the JL_GC_PUSH to the top of the function for
example).

Fixes #42346

@vtjnash vtjnash requested a review from Keno September 24, 2021 17:17
@jonas-schulze
Copy link
Contributor

I can confirm that this builds successfully on my system (#42346 (comment)).

@DilumAluthge
Copy link
Member

@jonas-schulze Just to clarify, this builds successfully on your system with Xcode 13, right?

@DilumAluthge
Copy link
Member

@vtjnash @Keno Looks like analyzegc is failing?

jl_current_task is not constant after this point in the function, so we
split the function so that the compiler won't try to optimize it
incorrectly (hoisting the JL_GC_PUSH to the top of the function for
example).

Fixes #42346
@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Sep 27, 2021
@IanButterworth IanButterworth merged commit 4dea1c4 into master Sep 28, 2021
@IanButterworth IanButterworth deleted the jn/42346 branch September 28, 2021 00:53
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Sep 28, 2021
@DilumAluthge
Copy link
Member

Does this need backporting?

@carlocab
Copy link
Contributor

I'd really appreciate it if this could get backported.

@DilumAluthge
Copy link
Member

I'm going to add the backport labels for 1.6 and 1.7. @vtjnash let me know if that's not correct.

@DilumAluthge DilumAluthge added backport 1.6 Change should be backported to release-1.6 backport 1.7 bugfix This change fixes an existing bug labels Sep 28, 2021
KristofferC pushed a commit that referenced this pull request Sep 29, 2021
jl_current_task is not constant after this point in the function, so we
split the function so that the compiler won't try to optimize it
incorrectly (hoisting the JL_GC_PUSH to the top of the function for
example).

Fixes #42346

(cherry picked from commit 4dea1c4)
@carlocab
Copy link
Contributor

Sorry -- I should've been more explicit. I'm currently experiencing build failures with Xcode 13 only on the v1.7.0-rc1 tag. v1.6.3 still builds on Xcode 13, so it might be that this is not needed for the 1.6 branch unless the change that broke things on Xcode 13 is backported there too.

@DilumAluthge DilumAluthge removed the backport 1.6 Change should be backported to release-1.6 label Sep 30, 2021
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
jl_current_task is not constant after this point in the function, so we
split the function so that the compiler won't try to optimize it
incorrectly (hoisting the JL_GC_PUSH to the top of the function for
example).

Fixes JuliaLang#42346
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
jl_current_task is not constant after this point in the function, so we
split the function so that the compiler won't try to optimize it
incorrectly (hoisting the JL_GC_PUSH to the top of the function for
example).

Fixes JuliaLang#42346
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Build fails on macOS with Xcode 13.0 (but passes with Xcode 12.5.1)
6 participants