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 codegen during sysimg restore. #41676

Merged
merged 1 commit into from
Jul 27, 2021
Merged

Init codegen during sysimg restore. #41676

merged 1 commit into from
Jul 27, 2021

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented Jul 22, 2021

Fixes #41670. Restores the old order of jl_init->jl_load_sysimg->jl_init_codegen, which was changed in https://github.com/JuliaLang/julia/pull/40715/files#diff-0e5a5cdf744ddbc1c815bf0a767bc9988e37d88792e8acc95c7ed9863491f2afL1797, because deserializing a system image might require the code generator to be initialized (deserializing ccallables calls jl_compile_extern_c).

@maleadt maleadt added bugfix This change fixes an existing bug backport 1.7 labels Jul 22, 2021
@maleadt maleadt requested a review from vtjnash July 22, 2021 08:57
@vtjnash
Copy link
Member

vtjnash commented Jul 22, 2021

The state of the sysimg is still indeterminate at this point. You cannot access it externally (in codegen) until it is finished.

@JeffBezanson
Copy link
Member

Ok, is there a way we can split up jl_init_codegen that would work?

@vtjnash
Copy link
Member

vtjnash commented Jul 22, 2021

init_codegen is fine (it is pretty much free of external state), the jl_compile_extern_c call is bad

@JeffBezanson
Copy link
Member

What state hasn't been set up at that point?

@maleadt
Copy link
Member Author

maleadt commented Jul 27, 2021

@vtjnash Care to elaborate? How else would you propose fixing this, try to make jl_compile_extern_c run later?

@vtjnash vtjnash merged commit 2fbeef8 into master Jul 27, 2021
@vtjnash vtjnash deleted the tb/init_codegen branch July 27, 2021 14:44
@vtjnash
Copy link
Member

vtjnash commented Jul 27, 2021

Jeff pointed out offline that this is happening after deserialize, but that it is just poorly named

@maleadt
Copy link
Member Author

maleadt commented Jul 27, 2021

OK, thanks!

KristofferC pushed a commit that referenced this pull request Aug 2, 2021
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.

jl_compile_extern_c appears to be running at the wrong time in sysimg startup
4 participants