Partially revert 44512 to handle duplicated spec_types inlining bug

Author: Ian Atol

base/compiler/ssair/inlining.jl

@@ -880,7 +880,7 @@ function validate_sparams(sparams::SimpleVector)
 end
 
 function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
-                         flag::UInt8, state::InliningState)
+                         flag::UInt8, state::InliningState, check_sparams::Bool=false)
     method = match.method
     spec_types = match.spec_types
 
@@ -902,7 +902,7 @@ function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
         end
     end
 
-    #validate_sparams(match.sparams) || return nothing
+    check_sparams && (validate_sparams(match.sparams) || return nothing)
 
     et = state.et
 
@@ -1238,6 +1238,9 @@ function analyze_single_call!(
     cases = InliningCase[]
     local any_fully_covered = false
     local handled_all_cases = true
+    local revisit_idx = nothing
+    local only_method = nothing
+    local meth::MethodLookupResult
     for i in 1:length(infos)
         meth = infos[i].results
         if meth.ambig
@@ -1248,14 +1251,64 @@ function analyze_single_call!(
             # No applicable methods; try next union split
             handled_all_cases = false
             continue
+        else
+            if length(meth) == 1 && only_method !== false
+                if only_method === nothing
+                    only_method = meth[1].method
+                elseif only_method !== meth[1].method
+                    only_method = false
+                end
+            else
+                only_method = false
+            end
         end
-        for match in meth
-            handled_all_cases &= handle_match!(match, argtypes, flag, state, cases, true)
+        for (j, match) in enumerate(meth)
+            if !isdispatchtuple(match.spec_types)
+                if !match.fully_covers
+                    handled_all_cases = false
+                    continue
+                end
+                if revisit_idx === nothing
+                    revisit_idx = (i, j)
+                else
+                    handled_all_cases = false
+                    revisit_idx = nothing
+                end
+            else
+                handled_all_cases &= handle_match!(match, argtypes, flag, state, cases, true)
+            end
             any_fully_covered |= match.fully_covers
         end
     end
 
-    if !handled_all_cases
+    atype = argtypes_to_type(argtypes)
+    if handled_all_cases && revisit_idx !== nothing
+        # If there's only one case that's not a dispatchtuple, we can
+        # still unionsplit by visiting all the other cases first.
+        # This is useful for code like:
+        #   foo(x::Int) = 1
+        #   foo(@nospecialize(x::Any)) = 2
+        # where we where only a small number of specific dispatchable
+        # cases are split off from an ::Any typed fallback.
+        (i, j) = revisit_idx
+        match = infos[i].results[j]
+        handled_all_cases &= handle_match!(match, argtypes, flag, state, cases, true, true)
+    elseif length(cases) == 0 && only_method isa Method
+        # if the signature is fully covered and there is only one applicable method,
+        # we can try to inline it even if the signature is not a dispatch tuple.
+        # -- But don't try it if we already tried to handle the match in the revisit_idx
+        # case, because that'll (necessarily) be the same method.
+        if length(infos) > 1
+            (metharg, methsp) = ccall(:jl_type_intersection_with_env, Any, (Any, Any),
+                atype, only_method.sig)::SimpleVector
+            match = MethodMatch(metharg, methsp::SimpleVector, only_method, true)
+        else
+            @assert length(meth) == 1
+            match = meth[1]
+        end
+        handle_match!(match, argtypes, flag, state, cases, true, false) || return nothing
+        any_fully_covered = handled_all_cases = match.fully_covers
+    elseif !handled_all_cases
         # if we've not seen all candidates, union split is valid only for dispatch tuples
         filter!(case::InliningCase->isdispatchtuple(case.sig), cases)
     end
@@ -1313,14 +1366,18 @@ end
 
 function handle_match!(
     match::MethodMatch, argtypes::Vector{Any}, flag::UInt8, state::InliningState,
-    cases::Vector{InliningCase}, allow_abstract::Bool = false)
+    cases::Vector{InliningCase}, allow_abstract::Bool = false, check_sparams::Bool=false)
     spec_types = match.spec_types
     allow_abstract || isdispatchtuple(spec_types) || return false
-    # we may see duplicated dispatch signatures here when a signature gets widened
-    # during abstract interpretation: for the purpose of inlining, we can just skip
-    # processing this dispatch candidate
-    _any(case->case.sig === spec_types, cases) && return true
-    item = analyze_method!(match, argtypes, flag, state)
+    if check_sparams
+        # we may see duplicated dispatch signatures here when a signature gets widened
+        # during abstract interpretation: for the purpose of inlining, we can just skip
+        # processing this dispatch candidate
+        _any(case->case.sig === spec_types, cases) && return true
+        item = analyze_method!(match, argtypes, flag, state, true)
+    else
+        item = analyze_method!(match, argtypes, flag, state)
+    end
     item === nothing && return false
     push!(cases, InliningCase(spec_types, item))
     return true

src/builtins.c

@@ -1614,10 +1614,11 @@ JL_CALLABLE(jl_f__svec_ref)
     JL_TYPECHK(_svec_ref, simplevector, (jl_value_t*)s);
     JL_TYPECHK(_svec_ref, long, i);
     ssize_t idx = jl_unbox_long(i);
-    if (idx < 1 || idx > jl_svec_len(s)) {
-        jl_bounds_error_int(s, i);
+    size_t len = jl_svec_len(s);
+    if (idx < 1 || idx > len) {
+        jl_bounds_error_int((jl_value_t*)s, idx);
     }
-    return jl_svec_ref(s, jl_unbox_long(i)-1);
+    return jl_svec_ref(s, idx-1);
 }
 
 static int equiv_field_types(jl_value_t *old, jl_value_t *ft)