From a3453441fcec816258dea81aef4d17b1e934b61a Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 8 Mar 2023 16:20:14 -0500 Subject: [PATCH] gf: add support for invalidating invoke edges Apparently we never actually implemented support for invalidation detection on invoke edges, and furthermore, it had erased part of the support for regular edges. Generalize our code for detecting replacement of a method, to be used when computing replacement of an invoke edge. Fix #48802 --- src/gf.c | 109 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 58 insertions(+), 51 deletions(-) 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); } } }