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: enable constant propagation for invoked calls #41024

Closed
aviatesk opened this issue May 31, 2021 · 0 comments · Fixed by #41383
Closed

inference: enable constant propagation for invoked calls #41024

aviatesk opened this issue May 31, 2021 · 0 comments · Fixed by #41383
Labels
compiler:inference Type inference

Comments

@aviatesk
Copy link
Member

aviatesk commented May 31, 2021

The simple but realistic target code would be like this:

# interface, which expects common field `x::Int`
abstract type AbstractInterface end

# to help inference of mixins
function Base.getproperty(x::AbstractInterface, sym::Symbol)
    if sym === :x
        return getfield(x, sym)::Int
    else
        return getfield(x, sym) # fallback
    end
end

# another interface, which expects additional field `y::Rational{Int}`
abstract type AbstractInterfaceExtended <: AbstractInterface end

# to help inference of mixins
function Base.getproperty(x::AbstractInterfaceExtended, sym::Symbol)
    if sym === :y
        return getfield(x, sym)::Rational{Int}
    end
    return Base.@invoke getproperty(x::AbstractInterface, sym::Symbol)
end

Constant prop' works well for a case when sym === :y:

julia> code_typed((AbstractInterfaceExtended,); optimize=false) do x
           x.y
       end |> first
CodeInfo(
1%1 = Base.getproperty(x, :y)::Rational{Int64}
└──      return %1
) => Rational{Int64}

but doens't work for cases when sym === :x:

julia> code_typed((AbstractInterfaceExtended,); optimize=false) do x
           x.x
       end |> first
CodeInfo(
1%1 = Base.getproperty(x, :x)::Any # we want ::Int here
└──      return %1
) => Any

julia> code_typed((AbstractInterface,); optimize=false) do x
           x.x
       end
1-element Vector{Any}:
 CodeInfo(
1%1 = Base.getproperty(x, :x)::Any # we want ::Int here
└──      return %1
) => Any

Simply we need to enable constant prop' for abstract_invoke.
Especially, we want to implement a way to recompute argtypes taking into account a invoked call signature computed by :jl_type_intersection_with_env:

(ti, env::SimpleVector) = ccall(:jl_type_intersection_with_env, Any, (Any, Any), nargtype, method.sig)::SimpleVector
rt, edge = typeinf_edge(interp, method, ti, env, sv)
edge !== nothing && add_backedge!(edge::MethodInstance, sv)
return CallMeta(rt, InvokeCallInfo(MethodMatch(ti, env, method, argtype <: method.sig)))

@aviatesk aviatesk added the compiler:inference Type inference label May 31, 2021
aviatesk added a commit that referenced this issue Jun 26, 2021


Especially useful for defining mixins with typed interface fields, e.g.
```julia
abstract type AbstractInterface end # mixin, which expects common field `x::Int`
function Base.getproperty(x::AbstractInterface, sym::Symbol)
    if sym === :x
        return getfield(x, sym)::Int # inferred field
    else
        return getfield(x, sym)      # fallback
    end
end

abstract type AbstractInterfaceExtended <: AbstractInterface end # extended mixin, which expects additional common field `y::Rational{Int}`
function Base.getproperty(x::AbstractInterfaceExtended, sym::Symbol)
    if sym === :y
        return getfield(x, sym)::Rational{Int}
    end
    return Base.@invoke getproperty(x::AbstractInterface, sym::Symbol)
end
```

As a bonus, inliner is able to use `InferenceResult` as a fast inlining
pass for constant-prop'ed `invoke`s
aviatesk added a commit that referenced this issue Jun 29, 2021


Especially useful for defining mixins with typed interface fields, e.g.
```julia
abstract type AbstractInterface end # mixin, which expects common field `x::Int`
function Base.getproperty(x::AbstractInterface, sym::Symbol)
    if sym === :x
        return getfield(x, sym)::Int # inferred field
    else
        return getfield(x, sym)      # fallback
    end
end

abstract type AbstractInterfaceExtended <: AbstractInterface end # extended mixin, which expects additional common field `y::Rational{Int}`
function Base.getproperty(x::AbstractInterfaceExtended, sym::Symbol)
    if sym === :y
        return getfield(x, sym)::Rational{Int}
    end
    return Base.@invoke getproperty(x::AbstractInterface, sym::Symbol)
end
```

As a bonus, inliner is able to use `InferenceResult` as a fast inlining
pass for constant-prop'ed `invoke`s
aviatesk added a commit that referenced this issue Jun 30, 2021
 (#41383)

* inference: enable constant propagation for `invoke`d calls, fixes #41024

Especially useful for defining mixins with typed interface fields, e.g.
```julia
abstract type AbstractInterface end # mixin, which expects common field `x::Int`
function Base.getproperty(x::AbstractInterface, sym::Symbol)
    if sym === :x
        return getfield(x, sym)::Int # inferred field
    else
        return getfield(x, sym)      # fallback
    end
end

abstract type AbstractInterfaceExtended <: AbstractInterface end # extended mixin, which expects additional common field `y::Rational{Int}`
function Base.getproperty(x::AbstractInterfaceExtended, sym::Symbol)
    if sym === :y
        return getfield(x, sym)::Rational{Int}
    end
    return Base.@invoke getproperty(x::AbstractInterface, sym::Symbol)
end
```

As a bonus, inliner is able to use `InferenceResult` as a fast inlining
pass for constant-prop'ed `invoke`s

* improve compile-time latency

* Update base/compiler/abstractinterpretation.jl

* Update base/compiler/abstractinterpretation.jl
KristofferC pushed a commit that referenced this issue Jun 30, 2021
 (#41383)

* inference: enable constant propagation for `invoke`d calls, fixes #41024

Especially useful for defining mixins with typed interface fields, e.g.
```julia
abstract type AbstractInterface end # mixin, which expects common field `x::Int`
function Base.getproperty(x::AbstractInterface, sym::Symbol)
    if sym === :x
        return getfield(x, sym)::Int # inferred field
    else
        return getfield(x, sym)      # fallback
    end
end

abstract type AbstractInterfaceExtended <: AbstractInterface end # extended mixin, which expects additional common field `y::Rational{Int}`
function Base.getproperty(x::AbstractInterfaceExtended, sym::Symbol)
    if sym === :y
        return getfield(x, sym)::Rational{Int}
    end
    return Base.@invoke getproperty(x::AbstractInterface, sym::Symbol)
end
```

As a bonus, inliner is able to use `InferenceResult` as a fast inlining
pass for constant-prop'ed `invoke`s

* improve compile-time latency

* Update base/compiler/abstractinterpretation.jl

* Update base/compiler/abstractinterpretation.jl

(cherry picked from commit bc6da93)
johanmon pushed a commit to johanmon/julia that referenced this issue Jul 5, 2021
…iaLang#41024 (JuliaLang#41383)

* inference: enable constant propagation for `invoke`d calls, fixes JuliaLang#41024

Especially useful for defining mixins with typed interface fields, e.g.
```julia
abstract type AbstractInterface end # mixin, which expects common field `x::Int`
function Base.getproperty(x::AbstractInterface, sym::Symbol)
    if sym === :x
        return getfield(x, sym)::Int # inferred field
    else
        return getfield(x, sym)      # fallback
    end
end

abstract type AbstractInterfaceExtended <: AbstractInterface end # extended mixin, which expects additional common field `y::Rational{Int}`
function Base.getproperty(x::AbstractInterfaceExtended, sym::Symbol)
    if sym === :y
        return getfield(x, sym)::Rational{Int}
    end
    return Base.@invoke getproperty(x::AbstractInterface, sym::Symbol)
end
```

As a bonus, inliner is able to use `InferenceResult` as a fast inlining
pass for constant-prop'ed `invoke`s

* improve compile-time latency

* Update base/compiler/abstractinterpretation.jl

* Update base/compiler/abstractinterpretation.jl
DilumAluthge added a commit that referenced this issue Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:inference Type inference
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant