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

Inference: propagate struct initialization info on setfield! #57222

Merged
merged 8 commits into from
Feb 8, 2025

Conversation

serenity4
Copy link
Contributor

When a variable has a field set with setfield!(var, field, value), inference now assumes that this specific field is defined and may for example constant-propagate isdefined(var, field) as true. PartialStruct, the lattice element used to encode this information, still has a few limitations in terms of what it may represent (it cannot represent mutable structs with non-contiguously defined fields yet), further work on extending it would increase the impact of this change.

Consider the following function:

julia> function f()
           a = A(1)
           setfield!(a, :y, 2)
           invokelatest(identity, a)
           isdefined(a, :y) && return 1.0
           a
       end
f (generic function with 1 method)

Here is before on master:

julia> @code_typed f()
CodeInfo(
1%1 = %new(Main.A, 1)::A
│          builtin Main.setfield!(%1, :y, 2)::Int64
│        dynamic builtin (Core._call_latest)(identity, %1)::Any%4 =   builtin Main.isdefined(%1, :y)::Bool
└──      goto #3 if not %4
2return 1.0
3return %1
) => Union{Float64, A}

And after this PR:

julia> @code_typed f()
CodeInfo(
1%1 = %new(Main.A, 1)::A
│          builtin Main.setfield!(%1, :y, 2)::Int64
│        dynamic builtin (Core._call_latest)(identity, %1)::Any
└──      return 1.0
) => Float64

@serenity4
Copy link
Contributor Author

The mechanism used to propagate PartialStruct refinement is frame-local however, and will currently not propagate if using setproperty! instead of setfield! in the example above (because setfield! is then inferred in the context of setproperty! and the refined type information is not saved once done with setproperty!).

@serenity4 serenity4 force-pushed the refine-struct-on-setfield branch from b6aeb98 to 9906616 Compare February 3, 2025 09:16
@serenity4 serenity4 marked this pull request as ready for review February 3, 2025 09:16
@oscardssmith oscardssmith added bugfix This change fixes an existing bug compiler:inference Type inference performance Must go faster labels Feb 5, 2025
@serenity4
Copy link
Contributor Author

This should be ready (if no new issues are detected in tests) @aviatesk

Copy link
Member

@aviatesk aviatesk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@aviatesk
Copy link
Member

aviatesk commented Feb 7, 2025

Restarted test revise. Once confirming it passes successfully, we can merge this.

@aviatesk aviatesk merged commit b682f49 into JuliaLang:master Feb 8, 2025
4 of 7 checks passed
topolarity pushed a commit that referenced this pull request Feb 25, 2025
…57304)

So far, `PartialStruct` has been unable to represent non-contiguously
defined fields, where e.g. a struct would have fields 1 and 3 defined
but not field 2. This PR extends it so that such information may be
represented with `PartialStruct`, extending the applicability of
optimizations e.g. introduced in #55297 by @aviatesk or #57222.

The semantics of `new` prevent the creation of a struct with
non-contiguously defined fields, therefore this change is mostly
relevant to model mutable structs whose fields may be previously set or
assumed to be defined after creation, or immutable structs whose
creation is opaque.

Notably, with this change we may now infer information about structs in
the following case:
```julia
mutable struct A; x; y; z; A() = new(); end

function f()
    mut = A()
   
    # some opaque call preventing optimizations
    # who knows, maybe `identity` will set fields from `mut` in a future world age!
    invokelatest(identity, mut)
   
    isdefined(mut, :z) && isdefined(mut, :x) || return
   
    isdefined(mut, :x) & isdefined(mut, :z) # this now infers as `true`
    isdefined(mut, :y) # this does not
end
```

whereas previously, only information gained successively with
`isdefined(mut, :x) && isdefined(mut, :y) && isdefined(mut, :z)` could
allow inference to model `mut` having its `z` field defined.

---------

Co-authored-by: Cédric Belmant <cedric.belmant@juliahub.com>
Co-authored-by: Shuhei Kadowaki <aviatesk@gmail.com>
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 compiler:inference Type inference performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants