Compiler: walk_to_defs, collect_leaves: specialize for predecessors

[nsajko]

Should make the code less vulnerable to invalidation.

[nsajko]

The invalidation count on loading TypeDomainNaturalNumbers v7.0.0 drops from 741 to 722. I haven't been able to get a nice diff, but these three method instances are some of those that are not invalidated on the PR branch:

{   
    "method_instance": {
        "method": "walk_to_defs(compact::Compiler.IncrementalCompact, defssa, typeconstraint, predecessors, 𝕃ₒ::Compiler.AbstractLattice) @ Compiler ../usr/share/julia/Compiler/src/ssair/passes.jl:274",
        "method_instance": "MethodInstance for Compiler.walk_to_defs(::Compiler.IncrementalCompact, ::Any, ::Any, ::Function, ::Compiler.PartialsLattice{Compiler.ConstsLattice})"
    },
    "children": [
        {   
            "method_instance": {
                "method": "collect_leaves(compact::Compiler.IncrementalCompact, val, typeconstraint, 𝕃ₒ::Compiler.AbstractLattice, predecessors) @ Compiler ../usr/share/julia/Compiler/src/ssair/passes.jl:185",
                "method_instance": "MethodInstance for Compiler.collect_leaves(::Compiler.IncrementalCompact, ::Any, ::Any, ::Compiler.PartialsLattice{Compiler.ConstsLattice}, ::Function)"
            },
            "children": [
                {   
                    "method_instance": {
                        "method": "lift_keyvalue_get!(compact::Compiler.IncrementalCompact, idx::Int64, stmt::Expr, 𝕃ₒ::Compiler.AbstractLattice) @ Compiler ../usr/share/julia/Compiler/src/ssair/passes.jl:981",
                        "method_instance": "MethodInstance for Compiler.lift_keyvalue_get!(::Compiler.IncrementalCompact, ::Int64, ::Expr, ::Compiler.PartialsLattice{Compiler.ConstsLattice})"
                    }
                }
            ]
        }
    ]
}

Data:

• master_e746505b7d.json
• pr.json

[KristofferC]

There is another length(values) a few lines above. Is that also not problematic?

(fast fingered the close button)

[topolarity]

The proper fix here to my eyes is that this should be specializing on the type of predecessors, so that values is then a well-inferred type

[vtjnash]

Looks like it is an argument, so that should be written , predecessors::P, ...) where P

[nsajko]

The proper fix here to my eyes is that this should be specializing on the type of predecessors

Just tried doing that while getting rid of the type assert. It gets rid of two additional invalidations, but sadly reintroduces the invalidations that were eliminated by the type assert.

I say merge this as-is, it's probably best to keep these PRs minimal to ease backporting or reverts if necessary.

The invalidations fixed by the current version of this PR:

{
    "method_instance": {
        "method": "isempty(r::StepRange) @ Base range.jl:685",
        "method_instance": "MethodInstance for isempty(::StepRange{Tuple{LineNumberNode, Expr}})"
    },
    "children": [
        {
            "method_instance": {
                "method": "iterate(r::OrdinalRange) @ Base range.jl:917",
                "method_instance": "MethodInstance for iterate(::OrdinalRange{Tuple{LineNumberNode, Expr}})"
            },
            "children": [
                {
                    "method_instance": {
                        "method": "Vector{T}(r::AbstractRange{T}) where T @ Base range.jl:1395",
                        "method_instance": "MethodInstance for Vector{Tuple{LineNumberNode, Expr}}(::AbstractRange{Tuple{LineNumberNode, Expr}})"
                    },
                    "children": [
                    ]
                }
            ]
        }
    ]
}

[topolarity]

Just tried doing that while getting rid of the type assert. It gets rid of two additional invalidations, but sadly reintroduces the invalidations that were eliminated by the type assert.

Thanks for checking! It looks like collect_leaves probably also needs the same specialization trick

[nsajko]

With the current version of this PR, the unique invalidation count when running the MWE below drops from 627 to 610. The diff happens to be very ugly, so I'm not pasting any individual wins.

MWE:

struct S <: Integer end
function Base.zero(::S) end
function Base.one(::S) end
let int_types = (Bool, Int8, UInt8, Int16, UInt16, Int, UInt, BigInt),
    bin_ops = (+, *)
    for int_type ∈ int_types
        T = Symbol(int_type)
        @eval begin
            function S(::$T) end
            function Base.$T(::S) end
            function Base.promote_rule(::Type{$T}, ::Type{S}) end
            function Base.promote_rule(::Type{S}, ::Type{$T}) end
        end
        for bin_op ∈ bin_ops
            op = Symbol(bin_op)
            @eval begin
                function Base.$op(::$T, ::S) end
                function Base.$op(::S, ::$T) end
            end
        end
    end
end

Data on each invalidation:

• master_626d541de61.json
• pr__nsajko__Compiler_ssair_passes_type_assert_Int_before_Colon.json

[topolarity]

Thanks!