Skip to content

improve inferred effects for reshape for Array #58672

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

Conversation

nsajko
Copy link
Contributor

@nsajko nsajko commented Jun 8, 2025

Use LazyString in throw_dmrsa.

Copy link

github-actions bot commented Jun 8, 2025

Hello! I am a bot.

Thank you for your pull request!

I have assigned @LilithHafner to this pull request.

@LilithHafner can either choose to review this pull request themselves, or they can choose to find someone else to review this pull request.

Note: If you are a Julia committer, please make sure that your organization membership is public.

@nsajko
Copy link
Contributor Author

nsajko commented Jun 8, 2025

FTR: the force push just added a @test, as can be seen in the Github Web UI, at least. I want to keep the PR in a single commit to prevent issues with possible backporting. I believe the PR should be backportable to v1.11 and v1.12, and want to fix the effect inference there too. EDIT: it backports cleanly to v1.12.

@nsajko
Copy link
Contributor Author

nsajko commented Jun 9, 2025

Before:

julia> VERSION
v"1.13.0-DEV.720"

julia> Base.infer_effects(reshape, Tuple{Vector{Float32}, Tuple{Int}})
(!c,!e,!n,!t,!s,!m,!u,+o,!r)

julia> Base.infer_effects(reshape, Tuple{Matrix{Float32}, Tuple{Int}})
(!c,!e,!n,!t,!s,!m,!u,+o,!r)

julia> Base.infer_effects(reshape, Tuple{Vector{Float32}, Tuple{Int, Int}})
(!c,!e,!n,!t,!s,!m,!u,+o,!r)

julia> Base.infer_effects(reshape, Tuple{Matrix{Float32}, Tuple{Int, Int}})
(!c,!e,!n,!t,!s,!m,!u,+o,!r)

After:

julia> VERSION
v"1.13.0-DEV.721"

julia> Base.infer_effects(reshape, Tuple{Vector{Float32}, Tuple{Int}})
(?c,+e,!n,+t,+s,?m,+u,+o,+r)

julia> Base.infer_effects(reshape, Tuple{Matrix{Float32}, Tuple{Int}})
(?c,+e,!n,+t,+s,?m,+u,+o,+r)

julia> Base.infer_effects(reshape, Tuple{Vector{Float32}, Tuple{Int, Int}})
(?c,+e,!n,+t,+s,?m,+u,+o,+r)

julia> Base.infer_effects(reshape, Tuple{Matrix{Float32}, Tuple{Int, Int}})
(?c,+e,!n,+t,+s,?m,+u,+o,+r)

@nsajko nsajko added the backport 1.12 Change should be backported to release-1.12 label Jun 9, 2025
Use `LazyString` in `throw_dmrsa`.
@nsajko nsajko force-pushed the better_effect_inference_for_reshape branch from bd8f634 to c78e984 Compare June 9, 2025 15:53
@oscardssmith oscardssmith added the merge me PR is reviewed. Merge when all tests are passing label Jun 9, 2025
@IanButterworth IanButterworth merged commit 1052201 into JuliaLang:master Jun 9, 2025
8 checks passed
@LilithHafner LilithHafner removed the merge me PR is reviewed. Merge when all tests are passing label Jun 9, 2025
@nsajko nsajko deleted the better_effect_inference_for_reshape branch June 10, 2025 02:29
KristofferC pushed a commit that referenced this pull request Jun 11, 2025
nilesh646 pushed a commit to nilesh646/julia that referenced this pull request Jun 17, 2025
@KristofferC KristofferC mentioned this pull request Jun 25, 2025
60 tasks
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] compiler:effects effect analysis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants