From 1fa21628773d1b68318d7e9fc64f35abdf513aee Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Thu, 6 Jun 2024 11:53:54 -0400 Subject: [PATCH] Track "permanently defined" status for bindings This change regains some inference power that we lost with #53750. It adds an `isdefined` bit to all bindings to track whether they were defined when their module was open. If so, we can be sure that they will be defined forever. --- base/compiler/abstractinterpretation.jl | 8 +-- base/compiler/optimize.jl | 3 +- base/compiler/tfuncs.jl | 4 +- src/codegen.cpp | 4 +- src/julia.h | 4 +- src/module.c | 94 +++++++++++++++++++------ 6 files changed, 86 insertions(+), 31 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 46e15d0c3ad79..78758fb2db50a 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -2599,7 +2599,7 @@ function abstract_eval_isdefined(interp::AbstractInterpreter, e::Expr, vtypes::U elseif isa(sym, GlobalRef) if InferenceParams(interp).assume_bindings_static rt = Const(isdefined_globalref(sym)) - elseif isdefinedconst_globalref(sym) + elseif permanently_isdefined_globalref(sym) rt = Const(true) else effects = Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE) @@ -2820,10 +2820,10 @@ function override_effects(effects::Effects, override::EffectsOverride) end isdefined_globalref(g::GlobalRef) = !iszero(ccall(:jl_globalref_boundp, Cint, (Any,), g)) -isdefinedconst_globalref(g::GlobalRef) = isconst(g) && isdefined_globalref(g) +permanently_isdefined_globalref(g::GlobalRef) = !iszero(ccall(:jl_globalref_permboundp, Cint, (Any,), g)) function abstract_eval_globalref_type(g::GlobalRef) - if isdefinedconst_globalref(g) + if isconst(g) && isdefined_globalref(g) return Const(ccall(:jl_get_globalref_value, Any, (Any,), g)) end ty = ccall(:jl_get_binding_type, Any, (Any, Any), g.mod, g.name) @@ -2849,7 +2849,7 @@ function abstract_eval_globalref(interp::AbstractInterpreter, g::GlobalRef, sv:: else rt = Union{} end - elseif isdefinedconst_globalref(g) + elseif permanently_isdefined_globalref(g) nothrow = true end return RTEffects(rt, nothrow ? Union{} : UndefVarError, Effects(EFFECTS_TOTAL; consistent, nothrow, inaccessiblememonly)) diff --git a/base/compiler/optimize.jl b/base/compiler/optimize.jl index 2f5459646dabc..ccbda7ccfb169 100644 --- a/base/compiler/optimize.jl +++ b/base/compiler/optimize.jl @@ -298,7 +298,8 @@ function stmt_effect_flags(𝕃ₒ::AbstractLattice, @nospecialize(stmt), @nospe isa(stmt, GotoNode) && return (true, false, true) isa(stmt, GotoIfNot) && return (true, false, ⊑(𝕃ₒ, argextype(stmt.cond, src), Bool)) if isa(stmt, GlobalRef) - nothrow = consistent = isdefinedconst_globalref(stmt) + nothrow = permanently_isdefined_globalref(stmt) + consistent = nothrow && isconst(stmt.mod, stmt.name) return (consistent, nothrow, nothrow) elseif isa(stmt, Expr) (; head, args) = stmt diff --git a/base/compiler/tfuncs.jl b/base/compiler/tfuncs.jl index b069e09214bab..7cb82ee95171d 100644 --- a/base/compiler/tfuncs.jl +++ b/base/compiler/tfuncs.jl @@ -398,7 +398,7 @@ end if a1 === Module hasintersect(widenconst(sym), Symbol) || return Bottom if isa(sym, Const) && isa(sym.val, Symbol) && isa(arg1, Const) && - isdefinedconst_globalref(GlobalRef(arg1.val::Module, sym.val::Symbol)) + permanently_isdefined_globalref(GlobalRef(arg1.val::Module, sym.val::Symbol)) return Const(true) end elseif isa(sym, Const) @@ -3041,7 +3041,7 @@ end if M isa Const && s isa Const M, s = M.val, s.val if M isa Module && s isa Symbol - return isdefinedconst_globalref(GlobalRef(M, s)) + return permanently_isdefined_globalref(GlobalRef(M, s)) end end return false diff --git a/src/codegen.cpp b/src/codegen.cpp index b378130eed007..3f1c714f9b30a 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -3216,7 +3216,7 @@ static jl_cgval_t emit_globalop(jl_codectx_t &ctx, jl_module_t *mod, jl_sym_t *s Value *bp = global_binding_pointer(ctx, mod, sym, &bnd, true); if (bp == NULL) return jl_cgval_t(); - if (bnd && !bnd->constp) { + if (bnd && !bnd->constp && !ctx.is_toplevel) { jl_value_t *ty = jl_atomic_load_relaxed(&bnd->ty); if (ty != nullptr) { const std::string fname = issetglobal ? "setglobal!" : isreplaceglobal ? "replaceglobal!" : isswapglobal ? "swapglobal!" : ismodifyglobal ? "modifyglobal!" : "setglobalonce!"; @@ -5613,7 +5613,7 @@ static jl_cgval_t emit_isdefined(jl_codectx_t &ctx, jl_value_t *sym) } jl_binding_t *bnd = jl_get_binding(modu, name); if (bnd) { - if (jl_atomic_load_acquire(&bnd->value) != NULL && bnd->constp) + if (jl_atomic_load_acquire(&bnd->value) != NULL && (bnd->constp || bnd->isdefined)) return mark_julia_const(ctx, jl_true); Value *bp = julia_binding_gv(ctx, bnd); bp = julia_binding_pvalue(ctx, bp); diff --git a/src/julia.h b/src/julia.h index 4534d00caa888..b0e72cd6948c9 100644 --- a/src/julia.h +++ b/src/julia.h @@ -639,7 +639,7 @@ typedef struct _jl_binding_t { uint8_t imported:1; uint8_t usingfailed:1; uint8_t deprecated:2; // 0=not deprecated, 1=renamed, 2=moved to another package - uint8_t padding:1; + uint8_t isdefined:1; // 0=value may be null, 1=value is non-null (and always will be) } jl_binding_t; typedef struct { @@ -1950,11 +1950,13 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_type(jl_module_t *m, jl_sym_t *var); JL_DLLEXPORT jl_binding_t *jl_get_binding_wr(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var); JL_DLLEXPORT jl_binding_t *jl_get_binding_for_method_def(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var); JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var); +JL_DLLEXPORT int jl_permboundp(jl_module_t *m, jl_sym_t *var); JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var); JL_DLLEXPORT int jl_binding_resolved_p(jl_module_t *m, jl_sym_t *var); JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var); JL_DLLEXPORT int jl_globalref_is_const(jl_globalref_t *gr); JL_DLLEXPORT int jl_globalref_boundp(jl_globalref_t *gr); +JL_DLLEXPORT int jl_globalref_permboundp(jl_globalref_t *gr); JL_DLLEXPORT jl_value_t *jl_get_globalref_value(jl_globalref_t *gr); JL_DLLEXPORT jl_value_t *jl_get_global(jl_module_t *m JL_PROPAGATES_ROOT, jl_sym_t *var); JL_DLLEXPORT void jl_set_global(jl_module_t *m JL_ROOTING_ARGUMENT, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT); diff --git a/src/module.c b/src/module.c index 9242a65950201..a163b6866db5f 100644 --- a/src/module.c +++ b/src/module.c @@ -182,7 +182,7 @@ static jl_binding_t *new_binding(jl_module_t *mod, jl_sym_t *name) b->imported = 0; b->deprecated = 0; b->usingfailed = 0; - b->padding = 0; + b->isdefined = 0; JL_GC_PUSH1(&b); b->globalref = jl_new_globalref(mod, name, b); JL_GC_POP(); @@ -191,29 +191,65 @@ static jl_binding_t *new_binding(jl_module_t *mod, jl_sym_t *name) extern jl_mutex_t jl_modules_mutex; +/** + * Return true if the provided module is "open". + * + * An open module is one that has not yet been serialized. + **/ +static int module_is_open(jl_module_t *m) +{ + JL_LOCK(&jl_modules_mutex); + int open = ptrhash_has(&jl_current_modules, (void*)m); + if (!open && jl_module_init_order != NULL) { + size_t i, l = jl_array_len(jl_module_init_order); + for (i = 0; i < l; i++) { + if (m == (jl_module_t*)jl_array_ptr_ref(jl_module_init_order, i)) { + open = 1; + break; + } + } + } + JL_UNLOCK(&jl_modules_mutex); + return open; +} + +/** + * Return true if modifications of this module's bindings are guaranteed not to + * experience "mutation tearing". + * + * Mutation tearing happens when the modification of an object in a dependency's + * pkgimage ends up not recorded in the pkgimage being generated - instead the + * mutation is dropped. + * + * That behavior exists because there is no coherent way to resolve modifications + * across pkgimages. Objects must be serialized exactly once across all pkgimages, + * and once serialized, mutations to an object live only as long as the current + * Julia session. + **/ +static int module_mutation_is_persistent(jl_module_t *m) +{ + // No tearing if we're not generating a pkgimage - the semantic "timeline" dies + // with the current Julia session + if (!jl_generating_output()) + return 1; + + // No tearing if we're generating the sysimage - no objects have been serialized yet + if (!jl_options.incremental) + return 1; + + // No tearing if the module/binding we're modifying have not been serialized yet + // (i.e. they are "open") + return module_is_open(m); +} + static void check_safe_newbinding(jl_module_t *m, jl_sym_t *var) { if (jl_current_task->ptls->in_pure_callback) jl_errorf("new globals cannot be created in a generated function"); - if (jl_options.incremental && jl_generating_output()) { - JL_LOCK(&jl_modules_mutex); - int open = ptrhash_has(&jl_current_modules, (void*)m); - if (!open && jl_module_init_order != NULL) { - size_t i, l = jl_array_len(jl_module_init_order); - for (i = 0; i < l; i++) { - if (m == (jl_module_t*)jl_array_ptr_ref(jl_module_init_order, i)) { - open = 1; - break; - } - } - } - JL_UNLOCK(&jl_modules_mutex); - if (!open) { - jl_errorf("Creating a new global in closed module `%s` (`%s`) breaks incremental compilation " - "because the side effects will not be permanent.", - jl_symbol_name(m->name), jl_symbol_name(var)); - } - } + if (!module_mutation_is_persistent(m)) + jl_errorf("Creating a new global in closed module `%s` (`%s`) breaks incremental compilation " + "because the side effects will not be permanent.", + jl_symbol_name(m->name), jl_symbol_name(var)); } static jl_module_t *jl_binding_dbgmodule(jl_binding_t *b, jl_module_t *m, jl_sym_t *var) JL_GLOBALLY_ROOTED; @@ -684,6 +720,12 @@ JL_DLLEXPORT int jl_boundp(jl_module_t *m, jl_sym_t *var) // unlike most queries return b && (jl_atomic_load(&b->value) != NULL); } +JL_DLLEXPORT int jl_permboundp(jl_module_t *m, jl_sym_t *var) +{ + jl_binding_t *b = jl_get_binding(m, var); + return b && (b->isdefined || b->constp) && (jl_atomic_load(&b->value) != NULL); +} + JL_DLLEXPORT int jl_defines_or_exports_p(jl_module_t *m, jl_sym_t *var) { jl_binding_t *b = jl_get_module_binding(m, var, 0); @@ -834,6 +876,13 @@ JL_DLLEXPORT int jl_globalref_boundp(jl_globalref_t *gr) return b && jl_atomic_load_relaxed(&b->value) != NULL; } +JL_DLLEXPORT int jl_globalref_permboundp(jl_globalref_t *gr) +{ + jl_binding_t *b = gr->binding; + b = jl_resolve_owner(b, gr->mod, gr->name, NULL); + return b && (b->isdefined || b->constp) && (jl_atomic_load_relaxed(&b->value) != NULL); +} + JL_DLLEXPORT int jl_is_const(jl_module_t *m, jl_sym_t *var) { jl_binding_t *b = jl_get_binding(m, var); @@ -925,8 +974,11 @@ jl_value_t *jl_check_binding_wr(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var JL_DLLEXPORT void jl_checked_assignment(jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *rhs) { + assert(rhs != NULL); if (jl_check_binding_wr(b, mod, var, rhs, 1) != NULL) { - jl_atomic_store_release(&b->value, rhs); + jl_value_t *old = jl_atomic_exchange(&b->value, rhs); + if (__unlikely(old == NULL) && module_is_open(mod)) + b->isdefined = 1; jl_gc_wb(b, rhs); } }