diff --git a/src/gf.c b/src/gf.c index 357487d723e9b..c3bc714e6d7fd 100644 --- a/src/gf.c +++ b/src/gf.c @@ -1817,6 +1817,36 @@ static int jl_type_intersection2(jl_value_t *t1, jl_value_t *t2, jl_value_t **is return 1; } +enum morespec_options { + morespec_unknown, + morespec_isnot, + morespec_is +}; + +// check if `type` is replacing `m` with an ambiguity here, given other methods in `d` that already match it +// precondition: type is not more specific than `m` +static int is_replacing(jl_value_t *type, jl_method_t *m, jl_method_t *const *d, size_t n, jl_value_t *isect, jl_value_t *isect2, char *morespec) +{ + size_t k; + for (k = 0; k < n; k++) { + jl_method_t *m2 = d[k]; + // see if m2 also fully covered this intersection + if (m == m2 || !(jl_subtype(isect, m2->sig) || (isect2 && jl_subtype(isect2, m2->sig)))) + continue; + if (morespec[k] == (char)morespec_unknown) + morespec[k] = (char)(jl_type_morespecific(m2->sig, type) ? morespec_is : morespec_isnot); + if (morespec[k] == (char)morespec_is) + // not actually shadowing this--m2 will still be better + return 0; + // since m2 was also a previous match over isect, + // see if m was also previously dominant over all m2 + if (!jl_type_morespecific(m->sig, m2->sig)) + // m and m2 were previously ambiguous over the full intersection of mi with type, and will still be ambiguous with type + return 0; + } + return 1; +} + JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method, jl_tupletype_t *simpletype) { JL_TIMING(ADD_METHOD); @@ -1851,7 +1881,7 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method oldvalue = get_intersect_matches(jl_atomic_load_relaxed(&mt->defs), newentry); int invalidated = 0; - jl_method_t **d; + jl_method_t *const *d; size_t j, n; if (oldvalue == NULL) { d = NULL; @@ -1880,6 +1910,7 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method // -> less specific or ambiguous with any one of them: can ignore the missing edge (not missing) // -> some may have been ambiguous: still are // -> some may have been called: they may be partly replaced (will be detected in the loop later) + // c.f. `is_replacing`, which is a similar query, but with an existing method match to compare against missing = 1; size_t j; for (j = 0; j < n; j++) { @@ -1914,11 +1945,6 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method } if (oldvalue) { oldmi = jl_alloc_vec_any(0); - enum morespec_options { - morespec_unknown, - morespec_isnot, - morespec_is - }; char *morespec = (char*)alloca(n); memset(morespec, morespec_unknown, n); for (j = 0; j < n; j++) { @@ -1935,6 +1961,11 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method continue; isect3 = jl_type_intersection(m->sig, (jl_value_t*)mi->specTypes); if (jl_type_intersection2(type, isect3, &isect, &isect2)) { + // TODO: this only checks pair-wise for ambiguities, but the ambiguities could arise from the interaction of multiple methods + // and thus might miss a case where we introduce an ambiguity between two existing methods + // We could instead work to sort this into 3 groups `morespecific .. ambiguous .. lesspecific`, with `type` in ambiguous, + // such that everything in `morespecific` dominates everything in `ambiguous`, and everything in `ambiguous` dominates everything in `lessspecific` + // And then compute where each isect falls, and whether it changed group--necessitating invalidation--or not. if (morespec[j] == (char)morespec_unknown) morespec[j] = (char)(jl_type_morespecific(m->sig, type) ? morespec_is : morespec_isnot); if (morespec[j] == (char)morespec_is) @@ -1943,62 +1974,38 @@ JL_DLLEXPORT void jl_method_table_insert(jl_methtable_t *mt, jl_method_t *method if (ambig == morespec_unknown) ambig = jl_type_morespecific(type, m->sig) ? morespec_is : morespec_isnot; // replacing a method--see if this really was the selected method previously - // over the intersection - if (ambig == morespec_isnot) { - size_t k; - for (k = 0; k < n; k++) { - jl_method_t *m2 = d[k]; - if (m == m2 || !(jl_subtype(isect, m2->sig) || (isect && jl_subtype(isect, m2->sig)))) - continue; - if (morespec[k] == (char)morespec_unknown) - morespec[k] = (char)(jl_type_morespecific(m2->sig, type) ? morespec_is : morespec_isnot); - if (morespec[k] == (char)morespec_is) - // not actually shadowing this--m2 will still be better - break; - // since m2 was also a previous match over isect, - // see if m was also previously dominant over all m2 - if (!jl_type_morespecific(m->sig, m2->sig)) - break; - } - if (k != n) - continue; - } - // Before deciding whether to invalidate `mi`, check each backedge for `invoke`s - if (mi->backedges) { - jl_array_t *backedges = mi->backedges; + // over the intersection (not ambiguous) and the new method will be selected now (morespec_is) + int replaced_dispatch = ambig == morespec_is || is_replacing(type, m, d, n, isect, isect2, morespec); + // found that this specialization dispatch got replaced by m + // call invalidate_backedges(&do_nothing_with_codeinst, mi, max_world, "jl_method_table_insert"); + // but ignore invoke-type edges + jl_array_t *backedges = mi->backedges; + if (backedges) { size_t ib = 0, insb = 0, nb = jl_array_len(backedges); jl_value_t *invokeTypes; jl_method_instance_t *caller; while (ib < nb) { ib = get_next_edge(backedges, ib, &invokeTypes, &caller); - if (!invokeTypes) { - // ordinary dispatch, invalidate + int replaced_edge; + if (invokeTypes) { + // n.b. normally we must have mi.specTypes <: invokeTypes <: m.sig (though it might not strictly hold), so we only need to check the other subtypes + replaced_edge = jl_subtype(invokeTypes, type) && (ambig == morespec_is || is_replacing(type, m, d, n, invokeTypes, NULL, morespec)); + } + else { + replaced_edge = replaced_dispatch; + } + if (replaced_edge) { invalidate_method_instance(&do_nothing_with_codeinst, caller, max_world, 1); invalidated = 1; - } else { - // invoke-dispatch, check invokeTypes for validity - struct jl_typemap_assoc search = {invokeTypes, method->primary_world, NULL, 0, ~(size_t)0}; - oldentry = jl_typemap_assoc_by_type(jl_atomic_load_relaxed(&mt->defs), &search, /*offs*/0, /*subtype*/0); - if (oldentry && oldentry->func.method == mi->def.method) { - // We can safely keep this method - jl_array_ptr_set(backedges, insb++, invokeTypes); - jl_array_ptr_set(backedges, insb++, caller); - } else { - invalidate_method_instance(&do_nothing_with_codeinst, caller, max_world, 1); - invalidated = 1; - } + } + else { + insb = set_next_edge(backedges, insb, invokeTypes, caller); } } jl_array_del_end(backedges, nb - insb); } - if (!mi->backedges || jl_array_len(mi->backedges) == 0) { - jl_array_ptr_1d_push(oldmi, (jl_value_t*)mi); - invalidate_external(mi, max_world); - if (mi->backedges) { - invalidated = 1; - invalidate_backedges(&do_nothing_with_codeinst, mi, max_world, "jl_method_table_insert"); - } - } + jl_array_ptr_1d_push(oldmi, (jl_value_t*)mi); + invalidate_external(mi, max_world); } } }