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.

Sign up for GitHub

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

Jump to bottom

Fix final gc lowering on dynamically sized allocation #48620

Merged
merged 2 commits into from
Feb 10, 2023
Merged

Fix final gc lowering on dynamically sized allocation #48620

merged 2 commits into from
Feb 10, 2023

Conversation

wsmoses
Copy link
Contributor

@wsmoses wsmoses commented Feb 10, 2023 •
edited
Loading

Resolves EnzymeAD/Enzyme.jl#553 and is hit during 1.9 on EnzymeAD/Enzyme.jl#597 aka #48621

cc @vchuravy @vtjnash

@wsmoses
Copy link
Contributor Author

wsmoses commented Feb 10, 2023

This should be backported to 1.9 and earlier versions as well.

@oscardssmith oscardssmith added backport 1.8 Change should be backported to release-1.8 backport 1.9 Change should be backported to release-1.9 labels Feb 10, 2023
@vchuravy
Copy link
Sponsor Member

Also can you try locally jl_gc_alloc

julia/src/gc.c

Line 3181 in 05b99af

JL_DLLEXPORT jl_value_t *(jl_gc_alloc)(jl_ptls_t ptls, size_t sz, void *ty)
directly? It is exported from libjulia-internal.

@wsmoses
Copy link
Contributor Author

wsmoses commented Feb 10, 2023

jl_gc_alloc succeeds, and now pushed code to use that.

@vchuravy vchuravy added the status:merge me PR is reviewed. Merge when all tests are passing label Feb 10, 2023
@vchuravy vchuravy merged commit 7e57dc7 into JuliaLang:master Feb 10, 2023
@oscardssmith oscardssmith removed the status:merge me PR is reviewed. Merge when all tests are passing label Feb 11, 2023
@KristofferC KristofferC mentioned this pull request Feb 20, 2023
Merged
50 tasks
KristofferC pushed a commit that referenced this pull request Feb 20, 2023
@wsmoses
Fix final gc lowering on dynamically sized allocation (#48620)
62bdb7d 
(cherry picked from commit 7e57dc7)
@KristofferC
Copy link
Sponsor Member

I backported this but got an error in the llvmpasses: https://buildkite.com/julialang/julia-release-1-dot-9/builds/101#01866edd-e69e-4681-81ce-5585968bd21b. I'll drop it from the backport branch for now and either @wsmoses and @vchuravy can push a correct backport to those branches.

vchuravy pushed a commit that referenced this pull request Mar 3, 2023
@wsmoses @vchuravy
Fix final gc lowering on dynamically sized allocation (#48620)
da64199 
(cherry picked from commit 7e57dc7)
KristofferC pushed a commit that referenced this pull request Mar 3, 2023
@wsmoses
Fix final gc lowering on dynamically sized allocation (#48620)
4710009 
(cherry picked from commit 7e57dc7)
@KristofferC KristofferC removed the backport 1.9 Change should be backported to release-1.9 label Mar 6, 2023
@vchuravy vchuravy mentioned this pull request Jun 18, 2024
Merged
@wsmoses wsmoses deleted the dynfinal branch June 18, 2024 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vchuravy vchuravy vchuravy approved these changes

Assignees
No one assigned
Labels
backport 1.8 Change should be backported to release-1.8
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

lowerGCAllocBytes LLVM error running tests
4 participants
@wsmoses @vchuravy @KristofferC @oscardssmith