From 7be953f07eede4a6e04195a47bd3eac1c140526b Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Sat, 15 Feb 2025 17:59:26 -0500 Subject: [PATCH 01/16] bpart: Skip implicit import reval if using'd export set is unchanged (#57421) When loading a pkgimage, the new bpart validation code needs to check if the export set of any using'd packages differs from what it would have been during precompile. This could e.g. happen if somebody (or Revise) eval'd a new `export` statement into a package that was `using`'d. However, this case is somewhat rare, so let's optimize it by keeping a bit in `Module` that keeps track of whether anything like that has happened and if not skipping the revalidation. This slightly improves pkgimage load time in the ordinary case. More optimizations to follow. (cherry picked from commit 39255d47db7657950ff1c82137ecec5a70bae622) --- src/julia.h | 2 ++ src/julia_internal.h | 13 ++++++++---- src/module.c | 11 ++++++----- src/staticdata.c | 47 +++++++++++++++++++++++++++++++------------- 4 files changed, 50 insertions(+), 23 deletions(-) diff --git a/src/julia.h b/src/julia.h index 5edbf4a1c8955..85b44278a8c1c 100644 --- a/src/julia.h +++ b/src/julia.h @@ -770,6 +770,8 @@ typedef struct _jl_module_t { int8_t infer; uint8_t istopmod; int8_t max_methods; + // If cleared no binding partition in this module has BINDING_FLAG_EXPORTED and min_world > jl_require_world. + _Atomic(int8_t) export_set_changed_since_require_world; jl_mutex_t lock; intptr_t hash; } jl_module_t; diff --git a/src/julia_internal.h b/src/julia_internal.h index 983df1ccdc4c7..da8525de1ea6b 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -923,6 +923,7 @@ extern JL_DLLEXPORT jl_module_t *jl_precompile_toplevel_module JL_GLOBALLY_ROOTE extern jl_genericmemory_t *jl_global_roots_list JL_GLOBALLY_ROOTED; extern jl_genericmemory_t *jl_global_roots_keyset JL_GLOBALLY_ROOTED; extern arraylist_t *jl_entrypoint_mis; +JL_DLLEXPORT extern size_t jl_require_world; JL_DLLEXPORT int jl_is_globally_rooted(jl_value_t *val JL_MAYBE_UNROOTED) JL_NOTSAFEPOINT; JL_DLLEXPORT jl_value_t *jl_as_global_root(jl_value_t *val, int insert) JL_GLOBALLY_ROOTED; extern jl_svec_t *precompile_field_replace JL_GLOBALLY_ROOTED; @@ -938,6 +939,14 @@ STATIC_INLINE int jl_bkind_is_some_import(enum jl_partition_kind kind) JL_NOTSAF return kind == BINDING_KIND_IMPLICIT || kind == BINDING_KIND_EXPLICIT || kind == BINDING_KIND_IMPORTED; } +STATIC_INLINE int jl_bkind_is_some_guard(enum jl_partition_kind kind) JL_NOTSAFEPOINT { + return kind == BINDING_KIND_FAILED || kind == BINDING_KIND_GUARD; +} + +STATIC_INLINE int jl_bkind_is_some_implicit(enum jl_partition_kind kind) JL_NOTSAFEPOINT { + return kind == BINDING_KIND_IMPLICIT || jl_bkind_is_some_guard(kind); +} + STATIC_INLINE int jl_bkind_is_some_constant(enum jl_partition_kind kind) JL_NOTSAFEPOINT { return kind == BINDING_KIND_CONST || kind == BINDING_KIND_CONST_IMPORT || kind == BINDING_KIND_UNDEF_CONST || kind == BINDING_KIND_BACKDATED_CONST; } @@ -946,10 +955,6 @@ STATIC_INLINE int jl_bkind_is_defined_constant(enum jl_partition_kind kind) JL_N return kind == BINDING_KIND_CONST || kind == BINDING_KIND_CONST_IMPORT || kind == BINDING_KIND_BACKDATED_CONST; } -STATIC_INLINE int jl_bkind_is_some_guard(enum jl_partition_kind kind) JL_NOTSAFEPOINT { - return kind == BINDING_KIND_FAILED || kind == BINDING_KIND_GUARD; -} - JL_DLLEXPORT jl_binding_partition_t *jl_get_binding_partition(jl_binding_t *b JL_PROPAGATES_ROOT, size_t world) JL_GLOBALLY_ROOTED; JL_DLLEXPORT jl_binding_partition_t *jl_get_binding_partition_all(jl_binding_t *b JL_PROPAGATES_ROOT, size_t min_world, size_t max_world) JL_GLOBALLY_ROOTED; diff --git a/src/module.c b/src/module.c index 604e0b8e659fc..b20337faad035 100644 --- a/src/module.c +++ b/src/module.c @@ -909,10 +909,7 @@ static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *asname, size_t new_world = jl_atomic_load_acquire(&jl_world_counter)+1; jl_binding_partition_t *btopart = jl_get_binding_partition(bto, new_world); enum jl_partition_kind btokind = jl_binding_kind(btopart); - if (btokind == BINDING_KIND_GUARD || - btokind == BINDING_KIND_IMPLICIT || - btokind == BINDING_KIND_FAILED) { - + if (jl_bkind_is_some_implicit(btokind)) { jl_binding_partition_t *new_bpart = jl_replace_binding_locked(bto, btopart, (jl_value_t*)b, (explici != 0) ? BINDING_KIND_IMPORTED : BINDING_KIND_EXPLICIT, new_world); if (jl_atomic_load_relaxed(&new_bpart->max_world) == ~(size_t)0) jl_add_binding_backedge(b, (jl_value_t*)bto); @@ -1016,7 +1013,7 @@ JL_DLLEXPORT void jl_module_using(jl_module_t *to, jl_module_t *from) if (tob) { jl_binding_partition_t *tobpart = jl_get_binding_partition(tob, new_world); enum jl_partition_kind kind = jl_binding_kind(tobpart); - if (kind == BINDING_KIND_IMPLICIT || jl_bkind_is_some_guard(kind)) { + if (jl_bkind_is_some_implicit(kind)) { jl_replace_binding_locked(tob, tobpart, NULL, BINDING_KIND_IMPLICIT_RECOMPUTE, new_world); } } @@ -1294,6 +1291,10 @@ JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked2(jl_binding_t *b, jl_atomic_store_relaxed(&new_bpart->next, old_bpart); jl_gc_wb_fresh(new_bpart, old_bpart); + if (((old_bpart->kind & BINDING_FLAG_EXPORTED) || (kind & BINDING_FLAG_EXPORTED)) && jl_require_world != ~(size_t)0) { + jl_atomic_store_release(&b->globalref->mod->export_set_changed_since_require_world, 1); + } + jl_atomic_store_release(&b->partitions, new_bpart); jl_gc_wb(b, new_bpart); JL_GC_POP(); diff --git a/src/staticdata.c b/src/staticdata.c index 3e916a93c6382..de87fe64dfc7f 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -1394,6 +1394,10 @@ static void jl_write_module(jl_serializer_state *s, uintptr_t item, jl_module_t } } assert(ios_pos(s->s) - reloc_offset == tot); + + // After reload, everything that has happened in this process happened semantically at + // (for .incremental) or before jl_require_world, so reset this flag. + jl_atomic_store_relaxed(&newm->export_set_changed_since_require_world, 0); } static void record_memoryref(jl_serializer_state *s, size_t reloc_offset, jl_genericmemoryref_t ref) { @@ -3537,32 +3541,31 @@ extern void export_jl_small_typeof(void); int IMAGE_NATIVE_CODE_TAINTED = 0; // TODO: This should possibly be in Julia -static void jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_t *bpart, size_t mod_idx) +static int jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_t *bpart, size_t mod_idx, int unchanged_implicit) { - if (jl_atomic_load_relaxed(&bpart->max_world) != ~(size_t)0) - return; + return 1; size_t raw_kind = bpart->kind; enum jl_partition_kind kind = (enum jl_partition_kind)(raw_kind & 0x0f); - if (!jl_bkind_is_some_import(kind)) - return; - jl_binding_t *imported_binding = (jl_binding_t*)bpart->restriction; - jl_binding_partition_t *latest_imported_bpart = jl_atomic_load_relaxed(&imported_binding->partitions); - if (!latest_imported_bpart) - return; - if (kind == BINDING_KIND_IMPLICIT || kind == BINDING_KIND_FAILED) { + if (!unchanged_implicit && jl_bkind_is_some_implicit(kind)) { jl_check_new_binding_implicit(bpart, b, NULL, jl_atomic_load_relaxed(&jl_world_counter)); bpart->kind |= (raw_kind & 0xf0); if (bpart->min_world > jl_require_world) goto invalidated; } + if (!jl_bkind_is_some_import(kind)) + return 1; + jl_binding_t *imported_binding = (jl_binding_t*)bpart->restriction; + jl_binding_partition_t *latest_imported_bpart = jl_atomic_load_relaxed(&imported_binding->partitions); + if (!latest_imported_bpart) + return 1; if (latest_imported_bpart->min_world <= bpart->min_world) { // Imported binding is still valid if ((kind == BINDING_KIND_EXPLICIT || kind == BINDING_KIND_IMPORTED) && external_blob_index((jl_value_t*)imported_binding) != mod_idx) { jl_add_binding_backedge(imported_binding, (jl_value_t*)b); } - return; + return 1; } else { // Binding partition was invalidated @@ -3580,13 +3583,14 @@ static void jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_ jl_binding_t *bedge = (jl_binding_t*)edge; if (!jl_atomic_load_relaxed(&bedge->partitions)) continue; - jl_validate_binding_partition(bedge, jl_atomic_load_relaxed(&bedge->partitions), mod_idx); + jl_validate_binding_partition(bedge, jl_atomic_load_relaxed(&bedge->partitions), mod_idx, 0); } } if (bpart->kind & BINDING_FLAG_EXPORTED) { jl_module_t *mod = b->globalref->mod; jl_sym_t *name = b->globalref->name; JL_LOCK(&mod->lock); + jl_atomic_store_release(&mod->export_set_changed_since_require_world, 1); if (mod->usings_backedges) { for (size_t i = 0; i < jl_array_len(mod->usings_backedges); i++) { jl_module_t *edge = (jl_module_t*)jl_array_ptr_ref(mod->usings_backedges, i); @@ -3596,12 +3600,24 @@ static void jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_ if (!jl_atomic_load_relaxed(&importee->partitions)) continue; JL_UNLOCK(&mod->lock); - jl_validate_binding_partition(importee, jl_atomic_load_relaxed(&importee->partitions), mod_idx); + jl_validate_binding_partition(importee, jl_atomic_load_relaxed(&importee->partitions), mod_idx, 0); JL_LOCK(&mod->lock); } } JL_UNLOCK(&mod->lock); + return 0; } + return 1; +} + +static int all_usings_unchanged_implicit(jl_module_t *mod) +{ + int unchanged_implicit = 1; + for (size_t i = 0; unchanged_implicit && i < module_usings_length(mod); i++) { + jl_module_t *usee = module_usings_getmod(mod, i); + unchanged_implicit &= !jl_atomic_load_acquire(&usee->export_set_changed_since_require_world); + } + return unchanged_implicit; } static void jl_restore_system_image_from_stream_(ios_t *f, jl_image_t *image, jl_array_t *depmods, uint64_t checksum, @@ -4063,12 +4079,15 @@ static void jl_restore_system_image_from_stream_(ios_t *f, jl_image_t *image, jl jl_module_t *mod = (jl_module_t*)obj; size_t mod_idx = external_blob_index((jl_value_t*)mod); jl_svec_t *table = jl_atomic_load_relaxed(&mod->bindings); + int unchanged_implicit = all_usings_unchanged_implicit(mod); for (size_t i = 0; i < jl_svec_len(table); i++) { jl_binding_t *b = (jl_binding_t*)jl_svecref(table, i); if ((jl_value_t*)b == jl_nothing) continue; jl_binding_partition_t *bpart = jl_atomic_load_relaxed(&b->partitions); - jl_validate_binding_partition(b, bpart, mod_idx); + if (!jl_validate_binding_partition(b, bpart, mod_idx, unchanged_implicit)) { + unchanged_implicit = all_usings_unchanged_implicit(mod); + } } } } From 5aec07fc9a9606608fb1c576f380af04f0b10416 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Sat, 15 Feb 2025 13:34:23 -0500 Subject: [PATCH 02/16] generated: Switch resolution module back to what it was before (#57419) This addresses post-commit review https://github.com/JuliaLang/julia/pull/57230#discussion_r1939750418. This change was left-over from before I decided to also change the type of the `source` argument (at which point `source.module` was unavailable in the function). This module was supposed to be the same, but it turns out that both the julia tests and several packages use this code manually and use different modules for the two places. Use the same one we used before (which is probably more correct anyway) to fix #57417 (cherry picked from commit 0c5372f68b6d80cd59ebb6d5410633a1fdfe011c) --- base/expr.jl | 2 +- test/staged.jl | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/base/expr.jl b/base/expr.jl index d71723ee26f1f..8ae394e122443 100644 --- a/base/expr.jl +++ b/base/expr.jl @@ -1694,6 +1694,6 @@ function (g::Core.GeneratedFunctionStub)(world::UInt, source::Method, @nospecial Expr(:meta, :pop_loc)))) spnames = g.spnames return generated_body_to_codeinfo(spnames === Core.svec() ? lam : Expr(Symbol("with-static-parameters"), lam, spnames...), - typename(typeof(g.gen)).module, + source.module, source.isva) end diff --git a/test/staged.jl b/test/staged.jl index 6811bc05a9b68..d416b0f9a22f0 100644 --- a/test/staged.jl +++ b/test/staged.jl @@ -449,3 +449,31 @@ end @test first(only(code_typed((Int,Int)) do x, y; @inline overdub54341(x, y); end)) isa Core.CodeInfo @test first(only(code_typed((Int,)) do x; @inline overdub54341(x, 1); end)) isa Core.CodeInfo @test_throws "Wrong number of arguments" overdub54341(1, 2, 3) + +# Test the module resolution scope of generated methods that are type constructors +module GeneratedScope57417 + using Test + import ..generate_lambda_ex + const x = 1 + struct Generator; end + @generated (::Generator)() = :x + f(x::Int) = 1 + module OtherModule + import ..f + const x = 2 + @generated f(::Float64) = :x + end + import .OtherModule: f + @test Generator()() == 1 + @test f(1.0) == 2 + + function g_generator(world::UInt, source::Method, _) + return generate_lambda_ex(world, source, (:g,), (), :(return x)) + end + + @eval function g() + $(Expr(:meta, :generated, g_generator)) + $(Expr(:meta, :generated_only)) + end + @test g() == 1 +end From 51fa78790b508ce4a2650293f0a232babb6c3077 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Sun, 16 Feb 2025 14:34:20 -0500 Subject: [PATCH 03/16] Prohibit binding replacement in closed modules during precompile (#57425) This applies the existing prohibition against introducing new bindings in a closed module to binding replacement as well (for the same reason - the change won't be persisted after reload). It is pretty hard to even reach this point, since `eval` into closed modules is already prohibited and there's no surface syntax for cross-module declaration, but it is technically reachable from lowered IR. Further, in the future we may make all of these builtins, which would make it easier. Thus, be consistent now and fully disallow binding replacement in closed modules during precompile. (cherry picked from commit 56aed6278d0037862d24743d7df3c3055b1360da) --- src/julia_internal.h | 2 ++ src/module.c | 21 +++++++++++++++++---- src/toplevel.c | 4 +++- test/precompile.jl | 21 +++++++++++++++++++++ 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/src/julia_internal.h b/src/julia_internal.h index da8525de1ea6b..dff1d90aecd2d 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -1185,7 +1185,9 @@ _Atomic(jl_value_t*) *jl_table_peek_bp(jl_genericmemory_t *a, jl_value_t *key) J JL_DLLEXPORT jl_method_t *jl_new_method_uninit(jl_module_t*); +JL_DLLEXPORT jl_module_t *jl_new_module__(jl_sym_t *name, jl_module_t *parent); JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, jl_module_t *parent, uint8_t default_using_core, uint8_t self_name); +JL_DLLEXPORT void jl_add_default_names(jl_module_t *m, uint8_t default_using_core, uint8_t self_name); JL_DLLEXPORT jl_methtable_t *jl_new_method_table(jl_sym_t *name, jl_module_t *module); JL_DLLEXPORT jl_method_instance_t *jl_get_specialization1(jl_tupletype_t *types JL_PROPAGATES_ROOT, size_t world, int mt_cache); jl_method_instance_t *jl_get_specialized(jl_method_t *m, jl_value_t *types, jl_svec_t *sp) JL_PROPAGATES_ROOT; diff --git a/src/module.c b/src/module.c index b20337faad035..7740a1bd252d0 100644 --- a/src/module.c +++ b/src/module.c @@ -221,7 +221,7 @@ jl_binding_partition_t *jl_get_binding_partition_all(jl_binding_t *b, size_t min return bpart; } -JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, jl_module_t *parent, uint8_t default_using_core, uint8_t self_name) +JL_DLLEXPORT jl_module_t *jl_new_module__(jl_sym_t *name, jl_module_t *parent) { jl_task_t *ct = jl_current_task; const jl_uuid_t uuid_zero = {0, 0}; @@ -253,7 +253,11 @@ JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, jl_module_t *parent, ui jl_atomic_store_relaxed(&m->bindings, jl_emptysvec); jl_atomic_store_relaxed(&m->bindingkeyset, (jl_genericmemory_t*)jl_an_empty_memory_any); arraylist_new(&m->usings, 0); - JL_GC_PUSH1(&m); + return m; +} + +JL_DLLEXPORT void jl_add_default_names(jl_module_t *m, uint8_t default_using_core, uint8_t self_name) +{ if (jl_core_module) { // Bootstrap: Before jl_core_module is defined, we don't have enough infrastructure // for bindings, so Core itself gets special handling in jltypes.c @@ -262,14 +266,22 @@ JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, jl_module_t *parent, ui } if (self_name) { // export own name, so "using Foo" makes "Foo" itself visible - jl_set_const(m, name, (jl_value_t*)m); - jl_module_public(m, name, 1); + jl_set_const(m, m->name, (jl_value_t*)m); + jl_module_public(m, m->name, 1); } } +} + +JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, jl_module_t *parent, uint8_t default_using_core, uint8_t self_name) +{ + jl_module_t *m = jl_new_module__(name, parent); + JL_GC_PUSH1(&m); + jl_add_default_names(m, default_using_core, self_name); JL_GC_POP(); return m; } + // Precondition: world_counter_lock is held JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val3( jl_binding_t *b, jl_module_t *mod, jl_sym_t *var, jl_value_t *val, @@ -1273,6 +1285,7 @@ JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked(jl_binding_t *b, JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked2(jl_binding_t *b, jl_binding_partition_t *old_bpart, jl_value_t *restriction_val, size_t kind, size_t new_world) { + check_safe_newbinding(b->globalref->mod, b->globalref->name); assert(jl_atomic_load_relaxed(&b->partitions) == old_bpart); jl_atomic_store_release(&old_bpart->max_world, new_world-1); jl_binding_partition_t *new_bpart = new_binding_partition(); diff --git a/src/toplevel.c b/src/toplevel.c index d931b59178ce6..dd9adb9bb480f 100644 --- a/src/toplevel.c +++ b/src/toplevel.c @@ -138,13 +138,15 @@ static jl_value_t *jl_eval_module_expr(jl_module_t *parent_module, jl_expr_t *ex // If we have `Base`, don't also try to import `Core` - the `Base` exports are a superset. // While we allow multiple imports of the same binding from different modules, various error printing // performs reflection on which module a binding came from and we'd prefer users see "Base" here. - jl_module_t *newm = jl_new_module_(name, is_parent__toplevel__ ? NULL : parent_module, std_imports && jl_base_module != NULL ? 0 : 1, 1); + jl_module_t *newm = jl_new_module__(name, is_parent__toplevel__ ? NULL : parent_module); jl_value_t *form = (jl_value_t*)newm; JL_GC_PUSH1(&form); JL_LOCK(&jl_modules_mutex); ptrhash_put(&jl_current_modules, (void*)newm, (void*)((uintptr_t)HT_NOTFOUND + 1)); JL_UNLOCK(&jl_modules_mutex); + jl_add_default_names(newm, std_imports && jl_base_module != NULL ? 0 : 1, 1); + jl_module_t *old_toplevel_module = jl_precompile_toplevel_module; // copy parent environment info into submodule diff --git a/test/precompile.jl b/test/precompile.jl index 6d106acc185f5..1f851b4d087ff 100644 --- a/test/precompile.jl +++ b/test/precompile.jl @@ -2311,4 +2311,25 @@ precompile_test_harness("llvmcall validation") do load_path end end +precompile_test_harness("BindingReplaceDisallow") do load_path + write(joinpath(load_path, "BindingReplaceDisallow.jl"), + """ + module BindingReplaceDisallow + const sinreplace = try + eval(Expr(:block, + Expr(:const, GlobalRef(Base, :sin), 1), + nothing)) + catch ex + ex isa ErrorException || rethrow() + ex + end + end + """) + ji, ofile = Base.compilecache(Base.PkgId("BindingReplaceDisallow")) + @eval using BindingReplaceDisallow + invokelatest() do + @test BindingReplaceDisallow.sinreplace.msg == "Creating a new global in closed module `Base` (`sin`) breaks incremental compilation because the side effects will not be permanent." + end +end + finish_precompile_test!() From 1c9d39d34decd68ebd44746002e4830803646c5d Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Sun, 16 Feb 2025 18:57:58 -0500 Subject: [PATCH 04/16] Prohibit `import`ing or `using` Main during incremental compilation (#57426) An upcoming optimization will skip most binding validation if no binding replacement has taken place in (sysimage, pkgimage) modules. However, as a special case, we would like to treat `Main` as a non-sysimage module because the addition of new bindings in `Main` is common and we would like this to not ruin the optimization. To make this legal, we have to prohibit `import`ing or `using` any `Main` bindings in pkgimages. I don't think anybody actually does this, particularly, since `Main` is not considered loading during precompile (so you have to use the main binding via (Core|Base|).Main), and I can't think of any good semantic reason to want to do this, but regardless, it does add additional restrictions to `using`/`import`, so I wanted to break it out into its own PR. (cherry picked from commit 726c816b9590d748345fb615b76b685c79eafd0d) --- src/module.c | 48 +++++++++++++++++++++++++++++----------------- test/precompile.jl | 28 +++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 18 deletions(-) diff --git a/src/module.c b/src/module.c index 7740a1bd252d0..56056cef96fe9 100644 --- a/src/module.c +++ b/src/module.c @@ -509,28 +509,31 @@ static jl_binding_t *new_binding(jl_module_t *mod, jl_sym_t *name) extern jl_mutex_t jl_modules_mutex; +static int is_module_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; +} + extern void check_safe_newbinding(jl_module_t *m, jl_sym_t *var) { if (jl_current_task->ptls->in_pure_callback) jl_errorf("new strong globals cannot be created in a generated function. Declare them outside using `global x::Any`."); - 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 (jl_options.incremental && jl_generating_output() && !is_module_open(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)); } } @@ -876,9 +879,17 @@ static void jl_binding_dep_message(jl_module_t *m, jl_sym_t *name, jl_binding_t JL_GC_POP(); } +JL_DLLEXPORT void check_safe_import_from(jl_module_t *m) +{ + if (jl_options.incremental && jl_generating_output() && m == jl_main_module) { + jl_errorf("Any `import` or `using` from `Main` is prohibited during incremental compilation."); + } +} + // NOTE: we use explici since explicit is a C++ keyword static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *asname, jl_sym_t *s, int explici) { + check_safe_import_from(from); jl_binding_t *b = jl_get_binding(from, s); jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); if (b->deprecated) { @@ -988,6 +999,7 @@ JL_DLLEXPORT void jl_module_using(jl_module_t *to, jl_module_t *from) { if (to == from) return; + check_safe_import_from(from); JL_LOCK(&world_counter_lock); JL_LOCK(&to->lock); for (size_t i = 0; i < module_usings_length(to); i++) { diff --git a/test/precompile.jl b/test/precompile.jl index 1f851b4d087ff..9fd588cc50808 100644 --- a/test/precompile.jl +++ b/test/precompile.jl @@ -2332,4 +2332,32 @@ precompile_test_harness("BindingReplaceDisallow") do load_path end end +precompile_test_harness("MainImportDisallow") do load_path + write(joinpath(load_path, "MainImportDisallow.jl"), + """ + module MainImportDisallow + const importvar = try + import Base.Main: cant_get_at_me + catch ex + ex isa ErrorException || rethrow() + ex + end + const usingmain = try + using Base.Main + catch ex + ex isa ErrorException || rethrow() + ex + end + # Import `Main` is permitted, because it does not look at bindings inside `Main` + import Base.Main + end + """) + ji, ofile = Base.compilecache(Base.PkgId("MainImportDisallow")) + @eval using MainImportDisallow + invokelatest() do + @test MainImportDisallow.importvar.msg == "Any `import` or `using` from `Main` is prohibited during incremental compilation." + @test MainImportDisallow.usingmain.msg == "Any `import` or `using` from `Main` is prohibited during incremental compilation." + end +end + finish_precompile_test!() From 5e3f967e9666879716627d014d6ff8c32719c631 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 17 Feb 2025 01:24:45 -0500 Subject: [PATCH 05/16] bpart: Track whether any binding replacement has happened in image modules (#57433) This implements the optimization proposed in #57426 by keeping track of whether any bindings were replaced in image modules (excluding `Main` as facilitated by #57426). In addition, we augment serialization to keep track of whether a method body contains any GlobalRefs that point to a loaded (system or package) image. If both of these flags are true, we can skip scanning the body of the method, since we know that we neither need to add any additional backedges nor were any of the referenced bindings invalidated. The performance impact on end-to-end load time is small, but measurable. Overall `@time using ModelingToolkit` consistently improves about 5% using this PR. However, I should note that using time is still about 40% slower than 1.11. This is not necessarily an Apples-to-Apples comparison as there were substantial other changes on 1.12 (as well as current load-time-tunings targeting older versions), but I wanted to put the number context. (cherry picked from commit f6e2b989de8ba14d9356ef065497fab8e20d66a6) --- base/client.jl | 8 +++- base/invalidation.jl | 18 ++++++--- base/runtime_internals.jl | 2 + base/staticdata.jl | 3 +- src/ircode.c | 16 +++++++- src/jltypes.c | 6 ++- src/julia.h | 2 + src/julia_internal.h | 1 + src/method.c | 6 +++ src/module.c | 9 +++++ src/staticdata.c | 17 ++++---- stdlib/Serialization/src/Serialization.jl | 5 ++- test/rebinding.jl | 49 +++++++++++++++++++++++ 13 files changed, 122 insertions(+), 20 deletions(-) diff --git a/base/client.jl b/base/client.jl index df0cf9043996e..9b7d80f51c219 100644 --- a/base/client.jl +++ b/base/client.jl @@ -265,7 +265,7 @@ function exec_options(opts) distributed_mode = (opts.worker == 1) || (opts.nprocs > 0) || (opts.machine_file != C_NULL) if distributed_mode let Distributed = require(PkgId(UUID((0x8ba89e20_285c_5b6f, 0x9357_94700520ee1b)), "Distributed")) - Core.eval(MainInclude, :(const Distributed = $Distributed)) + MainInclude.Distributed = Distributed Core.eval(Main, :(using Base.MainInclude.Distributed)) invokelatest(Distributed.process_opts, opts) end @@ -400,7 +400,7 @@ function load_InteractiveUtils(mod::Module=Main) try # TODO: we have to use require_stdlib here because it is a dependency of REPL, but we would sort of prefer not to let InteractiveUtils = require_stdlib(PkgId(UUID(0xb77e0a4c_d291_57a0_90e8_8db25a27a240), "InteractiveUtils")) - Core.eval(MainInclude, :(const InteractiveUtils = $InteractiveUtils)) + MainInclude.InteractiveUtils = InteractiveUtils end catch ex @warn "Failed to import InteractiveUtils into module $mod" exception=(ex, catch_backtrace()) @@ -535,6 +535,10 @@ The thrown errors are collected in a stack of exceptions. """ global err = nothing +# Used for memoizing require_stdlib of these modules +global InteractiveUtils::Module +global Distributed::Module + # weakly exposes ans and err variables to Main export ans, err end diff --git a/base/invalidation.jl b/base/invalidation.jl index 462e348e09038..fbf54cc2e24a6 100644 --- a/base/invalidation.jl +++ b/base/invalidation.jl @@ -180,26 +180,34 @@ function binding_was_invalidated(b::Core.Binding) b.partitions.min_world > unsafe_load(cglobal(:jl_require_world, UInt)) end -function scan_new_method!(methods_with_invalidated_source::IdSet{Method}, method::Method) +function scan_new_method!(methods_with_invalidated_source::IdSet{Method}, method::Method, image_backedges_only::Bool) isdefined(method, :source) || return + if image_backedges_only && !has_image_globalref(method) + return + end src = _uncompressed_ir(method) mod = method.module foreachgr(src) do gr::GlobalRef b = convert(Core.Binding, gr) - binding_was_invalidated(b) && push!(methods_with_invalidated_source, method) + if binding_was_invalidated(b) + # TODO: We could turn this into an addition if condition. For now, use it as a reasonably cheap + # additional consistency chekc + @assert !image_backedges_only + push!(methods_with_invalidated_source, method) + end maybe_add_binding_backedge!(b, method) end end -function scan_new_methods(extext_methods::Vector{Any}, internal_methods::Vector{Any}) +function scan_new_methods(extext_methods::Vector{Any}, internal_methods::Vector{Any}, image_backedges_only::Bool) methods_with_invalidated_source = IdSet{Method}() for method in internal_methods if isa(method, Method) - scan_new_method!(methods_with_invalidated_source, method) + scan_new_method!(methods_with_invalidated_source, method, image_backedges_only) end end for tme::Core.TypeMapEntry in extext_methods - scan_new_method!(methods_with_invalidated_source, tme.func::Method) + scan_new_method!(methods_with_invalidated_source, tme.func::Method, image_backedges_only) end return methods_with_invalidated_source end diff --git a/base/runtime_internals.jl b/base/runtime_internals.jl index 3a66dbda97477..2c7bfa70055ae 100644 --- a/base/runtime_internals.jl +++ b/base/runtime_internals.jl @@ -1656,3 +1656,5 @@ isempty(mt::Core.MethodTable) = (mt.defs === nothing) uncompressed_ir(m::Method) = isdefined(m, :source) ? _uncompressed_ir(m) : isdefined(m, :generator) ? error("Method is @generated; try `code_lowered` instead.") : error("Code for this Method is not available.") + +has_image_globalref(m::Method) = ccall(:jl_ir_flag_has_image_globalref, Bool, (Any,), m.source) diff --git a/base/staticdata.jl b/base/staticdata.jl index 45504622fe0ec..94526cc4f7bd3 100644 --- a/base/staticdata.jl +++ b/base/staticdata.jl @@ -25,7 +25,8 @@ end function insert_backedges(edges::Vector{Any}, ext_ci_list::Union{Nothing,Vector{Any}}, extext_methods::Vector{Any}, internal_methods::Vector{Any}) # determine which CodeInstance objects are still valid in our image # to enable any applicable new codes - methods_with_invalidated_source = Base.scan_new_methods(extext_methods, internal_methods) + backedges_only = unsafe_load(cglobal(:jl_first_image_replacement_world, UInt)) == typemax(UInt) + methods_with_invalidated_source = Base.scan_new_methods(extext_methods, internal_methods, backedges_only) stack = CodeInstance[] visiting = IdDict{CodeInstance,Int}() _insert_backedges(edges, stack, visiting, methods_with_invalidated_source) diff --git a/src/ircode.c b/src/ircode.c index 99c5833ac3be7..ddd5bb29fdfac 100644 --- a/src/ircode.c +++ b/src/ircode.c @@ -547,7 +547,7 @@ static void jl_encode_value_(jl_ircode_state *s, jl_value_t *v, int as_literal) } } -static jl_code_info_flags_t code_info_flags(uint8_t propagate_inbounds, uint8_t has_fcall, +static jl_code_info_flags_t code_info_flags(uint8_t propagate_inbounds, uint8_t has_fcall, uint8_t has_image_globalref, uint8_t nospecializeinfer, uint8_t isva, uint8_t inlining, uint8_t constprop, uint8_t nargsmatchesmethod, jl_array_t *ssaflags) @@ -555,6 +555,7 @@ static jl_code_info_flags_t code_info_flags(uint8_t propagate_inbounds, uint8_t jl_code_info_flags_t flags; flags.bits.propagate_inbounds = propagate_inbounds; flags.bits.has_fcall = has_fcall; + flags.bits.has_image_globalref = has_image_globalref; flags.bits.nospecializeinfer = nospecializeinfer; flags.bits.isva = isva; flags.bits.inlining = inlining; @@ -1036,7 +1037,7 @@ JL_DLLEXPORT jl_string_t *jl_compress_ir(jl_method_t *m, jl_code_info_t *code) }; uint8_t nargsmatchesmethod = code->nargs == m->nargs; - jl_code_info_flags_t flags = code_info_flags(code->propagate_inbounds, code->has_fcall, + jl_code_info_flags_t flags = code_info_flags(code->propagate_inbounds, code->has_fcall, code->has_image_globalref, code->nospecializeinfer, code->isva, code->inlining, code->constprop, nargsmatchesmethod, @@ -1134,6 +1135,7 @@ JL_DLLEXPORT jl_code_info_t *jl_uncompress_ir(jl_method_t *m, jl_code_instance_t code->constprop = flags.bits.constprop; code->propagate_inbounds = flags.bits.propagate_inbounds; code->has_fcall = flags.bits.has_fcall; + code->has_image_globalref = flags.bits.has_image_globalref; code->nospecializeinfer = flags.bits.nospecializeinfer; code->isva = flags.bits.isva; code->purity.bits = read_uint16(s.s); @@ -1228,6 +1230,16 @@ JL_DLLEXPORT uint8_t jl_ir_flag_has_fcall(jl_string_t *data) return flags.bits.has_fcall; } +JL_DLLEXPORT uint8_t jl_ir_flag_has_image_globalref(jl_string_t *data) +{ + if (jl_is_code_info(data)) + return ((jl_code_info_t*)data)->has_image_globalref; + assert(jl_is_string(data)); + jl_code_info_flags_t flags; + flags.packed = jl_string_data(data)[ir_offset_flags]; + return flags.bits.has_image_globalref; +} + JL_DLLEXPORT uint16_t jl_ir_inlining_cost(jl_string_t *data) { if (jl_is_code_info(data)) diff --git a/src/jltypes.c b/src/jltypes.c index 7f89e15aa38f3..2af9340d5bdef 100644 --- a/src/jltypes.c +++ b/src/jltypes.c @@ -3485,7 +3485,7 @@ void jl_init_types(void) JL_GC_DISABLED jl_code_info_type = jl_new_datatype(jl_symbol("CodeInfo"), core, jl_any_type, jl_emptysvec, - jl_perm_symsvec(22, + jl_perm_symsvec(23, "code", "debuginfo", "ssavaluetypes", @@ -3502,13 +3502,14 @@ void jl_init_types(void) JL_GC_DISABLED "nargs", "propagate_inbounds", "has_fcall", + "has_image_globalref", "nospecializeinfer", "isva", "inlining", "constprop", "purity", "inlining_cost"), - jl_svec(22, + jl_svec(23, jl_array_any_type, jl_debuginfo_type, jl_any_type, @@ -3527,6 +3528,7 @@ void jl_init_types(void) JL_GC_DISABLED jl_bool_type, jl_bool_type, jl_bool_type, + jl_bool_type, jl_uint8_type, jl_uint8_type, jl_uint16_type, diff --git a/src/julia.h b/src/julia.h index 85b44278a8c1c..6acd82fdd3401 100644 --- a/src/julia.h +++ b/src/julia.h @@ -312,6 +312,7 @@ typedef struct _jl_code_info_t { // various boolean properties: uint8_t propagate_inbounds; uint8_t has_fcall; + uint8_t has_image_globalref; uint8_t nospecializeinfer; uint8_t isva; // uint8 settings @@ -2263,6 +2264,7 @@ JL_DLLEXPORT jl_value_t *jl_compress_ir(jl_method_t *m, jl_code_info_t *code); JL_DLLEXPORT jl_code_info_t *jl_uncompress_ir(jl_method_t *m, jl_code_instance_t *metadata, jl_value_t *data); JL_DLLEXPORT uint8_t jl_ir_flag_inlining(jl_value_t *data) JL_NOTSAFEPOINT; JL_DLLEXPORT uint8_t jl_ir_flag_has_fcall(jl_value_t *data) JL_NOTSAFEPOINT; +JL_DLLEXPORT uint8_t jl_ir_flag_has_image_globalref(jl_value_t *data) JL_NOTSAFEPOINT; JL_DLLEXPORT uint16_t jl_ir_inlining_cost(jl_value_t *data) JL_NOTSAFEPOINT; JL_DLLEXPORT ssize_t jl_ir_nslots(jl_value_t *data) JL_NOTSAFEPOINT; JL_DLLEXPORT uint8_t jl_ir_slotflag(jl_value_t *data, size_t i) JL_NOTSAFEPOINT; diff --git a/src/julia_internal.h b/src/julia_internal.h index dff1d90aecd2d..782e0bd0b1a96 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -650,6 +650,7 @@ STATIC_INLINE jl_value_t *undefref_check(jl_datatype_t *dt, jl_value_t *v) JL_NO typedef struct { uint16_t propagate_inbounds:1; uint16_t has_fcall:1; + uint16_t has_image_globalref:1; uint16_t nospecializeinfer:1; uint16_t isva:1; uint16_t nargsmatchesmethod:1; diff --git a/src/method.c b/src/method.c index 6f53a6ff55c49..1fd36b102e0f6 100644 --- a/src/method.c +++ b/src/method.c @@ -486,6 +486,11 @@ jl_code_info_t *jl_new_code_info_from_ir(jl_expr_t *ir) is_flag_stmt = 1; else if (jl_is_expr(st) && ((jl_expr_t*)st)->head == jl_return_sym) jl_array_ptr_set(body, j, jl_new_struct(jl_returnnode_type, jl_exprarg(st, 0))); + else if (jl_is_globalref(st)) { + jl_globalref_t *gr = (jl_globalref_t*)st; + if (jl_object_in_image((jl_value_t*)gr->mod)) + li->has_image_globalref = 1; + } else { if (jl_is_expr(st) && ((jl_expr_t*)st)->head == jl_assign_sym) st = jl_exprarg(st, 1); @@ -593,6 +598,7 @@ JL_DLLEXPORT jl_code_info_t *jl_new_code_info_uninit(void) src->max_world = ~(size_t)0; src->propagate_inbounds = 0; src->has_fcall = 0; + src->has_image_globalref = 0; src->nospecializeinfer = 0; src->constprop = 0; src->inlining = 0; diff --git a/src/module.c b/src/module.c index 56056cef96fe9..ad4613c2d5c56 100644 --- a/src/module.c +++ b/src/module.c @@ -1294,10 +1294,19 @@ JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked(jl_binding_t *b, new_world); } +extern JL_DLLEXPORT _Atomic(size_t) jl_first_image_replacement_world; JL_DLLEXPORT jl_binding_partition_t *jl_replace_binding_locked2(jl_binding_t *b, jl_binding_partition_t *old_bpart, jl_value_t *restriction_val, size_t kind, size_t new_world) { check_safe_newbinding(b->globalref->mod, b->globalref->name); + + // Check if this is a replacing a binding in the system or a package image. + // Until the first such replacement, we can fast-path validation. + // For these purposes, we consider the `Main` module to be a non-sysimg module. + // This is legal, because we special case the `Main` in check_safe_import_from. + if (jl_object_in_image((jl_value_t*)b) && b->globalref->mod != jl_main_module && jl_atomic_load_relaxed(&jl_first_image_replacement_world) == ~(size_t)0) + jl_atomic_store_relaxed(&jl_first_image_replacement_world, new_world); + assert(jl_atomic_load_relaxed(&b->partitions) == old_bpart); jl_atomic_store_release(&old_bpart->max_world, new_world-1); jl_binding_partition_t *new_bpart = new_binding_partition(); diff --git a/src/staticdata.c b/src/staticdata.c index de87fe64dfc7f..d69880aa20f71 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -90,6 +90,7 @@ External links: static const size_t WORLD_AGE_REVALIDATION_SENTINEL = 0x1; JL_DLLEXPORT size_t jl_require_world = ~(size_t)0; +JL_DLLEXPORT _Atomic(size_t) jl_first_image_replacement_world = ~(size_t)0; #include "staticdata_utils.c" #include "precompile_utils.c" @@ -3541,7 +3542,7 @@ extern void export_jl_small_typeof(void); int IMAGE_NATIVE_CODE_TAINTED = 0; // TODO: This should possibly be in Julia -static int jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_t *bpart, size_t mod_idx, int unchanged_implicit) +static int jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_t *bpart, size_t mod_idx, int unchanged_implicit, int no_replacement) { if (jl_atomic_load_relaxed(&bpart->max_world) != ~(size_t)0) return 1; @@ -3556,10 +3557,13 @@ static int jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_t if (!jl_bkind_is_some_import(kind)) return 1; jl_binding_t *imported_binding = (jl_binding_t*)bpart->restriction; + if (no_replacement) + goto add_backedge; jl_binding_partition_t *latest_imported_bpart = jl_atomic_load_relaxed(&imported_binding->partitions); if (!latest_imported_bpart) return 1; if (latest_imported_bpart->min_world <= bpart->min_world) { +add_backedge: // Imported binding is still valid if ((kind == BINDING_KIND_EXPLICIT || kind == BINDING_KIND_IMPORTED) && external_blob_index((jl_value_t*)imported_binding) != mod_idx) { @@ -3583,7 +3587,7 @@ static int jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_t jl_binding_t *bedge = (jl_binding_t*)edge; if (!jl_atomic_load_relaxed(&bedge->partitions)) continue; - jl_validate_binding_partition(bedge, jl_atomic_load_relaxed(&bedge->partitions), mod_idx, 0); + jl_validate_binding_partition(bedge, jl_atomic_load_relaxed(&bedge->partitions), mod_idx, 0, 0); } } if (bpart->kind & BINDING_FLAG_EXPORTED) { @@ -3600,7 +3604,7 @@ static int jl_validate_binding_partition(jl_binding_t *b, jl_binding_partition_t if (!jl_atomic_load_relaxed(&importee->partitions)) continue; JL_UNLOCK(&mod->lock); - jl_validate_binding_partition(importee, jl_atomic_load_relaxed(&importee->partitions), mod_idx, 0); + jl_validate_binding_partition(importee, jl_atomic_load_relaxed(&importee->partitions), mod_idx, 0, 0); JL_LOCK(&mod->lock); } } @@ -4070,8 +4074,7 @@ static void jl_restore_system_image_from_stream_(ios_t *f, jl_image_t *image, jl } } if (s.incremental) { - // This needs to be done in a second pass after the binding partitions - // have the proper ABI again. + int no_replacement = jl_atomic_load_relaxed(&jl_first_image_replacement_world) == ~(size_t)0; for (size_t i = 0; i < s.fixup_objs.len; i++) { uintptr_t item = (uintptr_t)s.fixup_objs.items[i]; jl_value_t *obj = (jl_value_t*)(image_base + item); @@ -4079,13 +4082,13 @@ static void jl_restore_system_image_from_stream_(ios_t *f, jl_image_t *image, jl jl_module_t *mod = (jl_module_t*)obj; size_t mod_idx = external_blob_index((jl_value_t*)mod); jl_svec_t *table = jl_atomic_load_relaxed(&mod->bindings); - int unchanged_implicit = all_usings_unchanged_implicit(mod); + int unchanged_implicit = no_replacement || all_usings_unchanged_implicit(mod); for (size_t i = 0; i < jl_svec_len(table); i++) { jl_binding_t *b = (jl_binding_t*)jl_svecref(table, i); if ((jl_value_t*)b == jl_nothing) continue; jl_binding_partition_t *bpart = jl_atomic_load_relaxed(&b->partitions); - if (!jl_validate_binding_partition(b, bpart, mod_idx, unchanged_implicit)) { + if (!jl_validate_binding_partition(b, bpart, mod_idx, unchanged_implicit, no_replacement)) { unchanged_implicit = all_usings_unchanged_implicit(mod); } } diff --git a/stdlib/Serialization/src/Serialization.jl b/stdlib/Serialization/src/Serialization.jl index bc476181e5b0d..3362c9439d385 100644 --- a/stdlib/Serialization/src/Serialization.jl +++ b/stdlib/Serialization/src/Serialization.jl @@ -80,7 +80,7 @@ const TAGS = Any[ const NTAGS = length(TAGS) @assert NTAGS == 255 -const ser_version = 29 # do not make changes without bumping the version #! +const ser_version = 30 # do not make changes without bumping the version #! format_version(::AbstractSerializer) = ser_version format_version(s::Serializer) = s.version @@ -1268,6 +1268,9 @@ function deserialize(s::AbstractSerializer, ::Type{CodeInfo}) if format_version(s) >= 20 ci.has_fcall = deserialize(s) end + if format_version(s) >= 30 + ci.has_image_globalref = deserialize(s)::Bool + end if format_version(s) >= 24 ci.nospecializeinfer = deserialize(s)::Bool end diff --git a/test/rebinding.jl b/test/rebinding.jl index 2f343fd86eb9a..c6feabdd13b59 100644 --- a/test/rebinding.jl +++ b/test/rebinding.jl @@ -221,3 +221,52 @@ module Regression end @test GeoParams57377.B.C.h() == GeoParams57377.B.C.S() end + +# Test that the validation bypass fast path is not defeated by loading InteractiveUtils +@test parse(UInt, readchomp(`$(Base.julia_cmd()) -e 'using InteractiveUtils; show(unsafe_load(cglobal(:jl_first_image_replacement_world, UInt)))'`)) == typemax(UInt) + +# Test that imported module binding backedges are still added in a new module that has the fast path active +let test_code = + """ + using Test + @assert unsafe_load(cglobal(:jl_first_image_replacement_world, UInt)) == typemax(UInt) + include("precompile_utils.jl") + + precompile_test_harness("rebinding precompile") do load_path + write(joinpath(load_path, "LotsOfBindingsToDelete2.jl"), + "module LotsOfBindingsToDelete2 + const delete_me_6 = 6 + end") + Base.compilecache(Base.PkgId("LotsOfBindingsToDelete2")) + write(joinpath(load_path, "UseTheBindings2.jl"), + "module UseTheBindings2 + import LotsOfBindingsToDelete2: delete_me_6 + f_use_bindings6() = delete_me_6 + # Code Instances for each of these + @assert (f_use_bindings6(),) == (6,) + end") + Base.compilecache(Base.PkgId("UseTheBindings2")) + @eval using LotsOfBindingsToDelete2 + @eval using UseTheBindings2 + invokelatest() do + @test UseTheBindings2.f_use_bindings6() == 6 + Base.delete_binding(LotsOfBindingsToDelete2, :delete_me_6) + invokelatest() do + @test_throws UndefVarError UseTheBindings2.f_use_bindings6() + end + end + end + + finish_precompile_test!() + """ + @test success(pipeline(`$(Base.julia_cmd()) -e $test_code`; stderr)) +end + +# Image Globalref smoke test +module ImageGlobalRefFlag + using Test + @eval fimage() = $(GlobalRef(Base, :sin)) + fnoimage() = x + @test Base.has_image_globalref(first(methods(fimage))) + @test !Base.has_image_globalref(first(methods(fnoimage))) +end From 5518d11ea41134956842b757d0290ac4d0ed0045 Mon Sep 17 00:00:00 2001 From: Daniel Karrasch Date: Thu, 20 Feb 2025 19:08:00 +0100 Subject: [PATCH 06/16] Update LinearAlgebra.version: redirect to the `release-1.12` branch --- stdlib/LinearAlgebra.version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stdlib/LinearAlgebra.version b/stdlib/LinearAlgebra.version index 96c00ca746651..7b2b896063c56 100644 --- a/stdlib/LinearAlgebra.version +++ b/stdlib/LinearAlgebra.version @@ -1,4 +1,4 @@ -LINEARALGEBRA_BRANCH = master +LINEARALGEBRA_BRANCH = release-1.12 LINEARALGEBRA_SHA1 = e7da19f2764ba36bd0a9eb8ec67dddce19d87114 LINEARALGEBRA_GIT_URL := https://github.com/JuliaLang/LinearAlgebra.jl.git LINEARALGEBRA_TAR_URL = https://api.github.com/repos/JuliaLang/LinearAlgebra.jl/tarball/$1 From 83e99fe08c89bc7f48f4f6f7b516bd0b32cdd147 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Fri, 7 Feb 2025 15:09:29 -0500 Subject: [PATCH 07/16] Add explicit imports for types and fix bugs (#57302) I was playing with strengthening the semantics around #57290. However, the particular change I was trying turned out too breaking (I may try a weaker version of it). Still, there were a number of good changes found in two categories: 1. We should explicitly import types when defining constructors. This has been allowed for a long time, but we may want to consider removing it, especially given the new binding semantics which make it more confusing as in #57290. 2. There were a couple of places where I don't think it was intended for generic functions in question not to extend Base. (cherry picked from commit 778e07906923fdf4966000983135901e11540109) --- Compiler/src/Compiler.jl | 2 +- base/Base_compiler.jl | 1 + base/bool.jl | 2 ++ base/char.jl | 2 ++ base/coreir.jl | 2 +- base/deprecated.jl | 1 + base/filesystem.jl | 6 +++--- base/float.jl | 3 +++ base/gmp.jl | 2 ++ base/hamt.jl | 10 +++++----- base/mpfr.jl | 5 ++++- base/multidimensional.jl | 4 +++- base/namedtuple.jl | 2 ++ base/refpointer.jl | 2 ++ base/strings/basic.jl | 2 ++ base/tuple.jl | 2 ++ 16 files changed, 36 insertions(+), 12 deletions(-) diff --git a/Compiler/src/Compiler.jl b/Compiler/src/Compiler.jl index ba6d1607ded55..2c68729ee1dc2 100644 --- a/Compiler/src/Compiler.jl +++ b/Compiler/src/Compiler.jl @@ -72,7 +72,7 @@ using Base.Order import Base: ==, _topmod, append!, convert, copy, copy!, findall, first, get, get!, getindex, haskey, in, isempty, isready, iterate, iterate, last, length, max_world, - min_world, popfirst!, push!, resize!, setindex!, size + min_world, popfirst!, push!, resize!, setindex!, size, intersect const getproperty = Core.getfield const setproperty! = Core.setfield! diff --git a/base/Base_compiler.jl b/base/Base_compiler.jl index 5655d9bbedbab..a1ae14427e754 100644 --- a/base/Base_compiler.jl +++ b/base/Base_compiler.jl @@ -227,6 +227,7 @@ function cld end function fld end # Lazy strings +import Core: String include("strings/lazy.jl") # array structures diff --git a/base/bool.jl b/base/bool.jl index 3a5c36b09ae2c..3658318d158e5 100644 --- a/base/bool.jl +++ b/base/bool.jl @@ -1,5 +1,7 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license +import Core: Bool + # promote Bool to any other numeric type promote_rule(::Type{Bool}, ::Type{T}) where {T<:Number} = T diff --git a/base/char.jl b/base/char.jl index 2e8410f6903e2..c089262ebf779 100644 --- a/base/char.jl +++ b/base/char.jl @@ -1,5 +1,7 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license +import Core: AbstractChar, Char + """ The `AbstractChar` type is the supertype of all character implementations in Julia. A character represents a Unicode code point, and can be converted diff --git a/base/coreir.jl b/base/coreir.jl index 5199dfd35f028..59422afb44add 100644 --- a/base/coreir.jl +++ b/base/coreir.jl @@ -49,5 +49,5 @@ while processing a call, then `Conditional` everywhere else. """ Core.InterConditional -InterConditional(var::SlotNumber, @nospecialize(thentype), @nospecialize(elsetype)) = +Core.InterConditional(var::SlotNumber, @nospecialize(thentype), @nospecialize(elsetype)) = InterConditional(slot_id(var), thentype, elsetype) diff --git a/base/deprecated.jl b/base/deprecated.jl index f72698ad47008..eeb7c0e60638e 100644 --- a/base/deprecated.jl +++ b/base/deprecated.jl @@ -353,6 +353,7 @@ end @deprecate one(i::CartesianIndex) oneunit(i) @deprecate one(I::Type{CartesianIndex{N}}) where {N} oneunit(I) +import .MPFR: BigFloat @deprecate BigFloat(x, prec::Int) BigFloat(x; precision=prec) @deprecate BigFloat(x, prec::Int, rounding::RoundingMode) BigFloat(x, rounding; precision=prec) @deprecate BigFloat(x::Real, prec::Int) BigFloat(x; precision=prec) diff --git a/base/filesystem.jl b/base/filesystem.jl index bc1f4942877e8..87b3552c80a2f 100644 --- a/base/filesystem.jl +++ b/base/filesystem.jl @@ -137,11 +137,11 @@ export File, S_IROTH, S_IWOTH, S_IXOTH, S_IRWXO import .Base: - IOError, _UVError, _sizeof_uv_fs, check_open, close, eof, eventloop, fd, isopen, - bytesavailable, position, read, read!, readavailable, seek, seekend, show, + IOError, _UVError, _sizeof_uv_fs, check_open, close, closewrite, eof, eventloop, fd, isopen, + bytesavailable, position, read, read!, readbytes!, readavailable, seek, seekend, show, skip, stat, unsafe_read, unsafe_write, write, transcode, uv_error, setup_stdio, rawhandle, OS_HANDLE, INVALID_OS_HANDLE, windowserror, filesize, - isexecutable, isreadable, iswritable, MutableDenseArrayType + isexecutable, isreadable, iswritable, MutableDenseArrayType, truncate import .Base.RefValue diff --git a/base/float.jl b/base/float.jl index faded5cd5978c..54e232a01b8cb 100644 --- a/base/float.jl +++ b/base/float.jl @@ -2,6 +2,9 @@ const IEEEFloat = Union{Float16, Float32, Float64} +import Core: Float16, Float32, Float64, AbstractFloat +import Core: Int8, Int16, Int32, Int64, Int128, UInt8, UInt16, UInt32, UInt64, UInt128 + ## floating point traits ## """ diff --git a/base/gmp.jl b/base/gmp.jl index 4d2b4b66ac41b..97488551f60f6 100644 --- a/base/gmp.jl +++ b/base/gmp.jl @@ -13,6 +13,8 @@ import .Base: *, +, -, /, <, <<, >>, >>>, <=, ==, >, >=, ^, (~), (&), (|), xor, sign, hastypemax, isodd, iseven, digits!, hash, hash_integer, top_set_bit, clamp, unsafe_takestring +import Core: Signed, Float16, Float32, Float64 + if Clong == Int32 const ClongMax = Union{Int8, Int16, Int32} const CulongMax = Union{UInt8, UInt16, UInt32} diff --git a/base/hamt.jl b/base/hamt.jl index e3e4b4bd03ba9..c77c592b17e58 100644 --- a/base/hamt.jl +++ b/base/hamt.jl @@ -239,11 +239,11 @@ or grows the HAMT by inserting a new trie instead. end end -length(::Leaf) = 1 -length(trie::HAMT) = sum((length(trie.data[i]) for i in eachindex(trie.data)), init=0) +Base.length(::Leaf) = 1 +Base.length(trie::HAMT) = sum((length(trie.data[i]) for i in eachindex(trie.data)), init=0) -isempty(::Leaf) = false -function isempty(trie::HAMT) +Base.isempty(::Leaf) = false +function Base.isempty(trie::HAMT) if islevel_empty(trie) return true end @@ -251,7 +251,7 @@ function isempty(trie::HAMT) end # DFS -function iterate(trie::HAMT, state=nothing) +function Base.iterate(trie::HAMT, state=nothing) if state === nothing state = (;parent=nothing, trie, i=1) end diff --git a/base/mpfr.jl b/base/mpfr.jl index 933e8eb46fb27..0d52510447b2f 100644 --- a/base/mpfr.jl +++ b/base/mpfr.jl @@ -20,12 +20,15 @@ import sinpi, cospi, sincospi, tanpi, sind, cosd, tand, asind, acosd, atand, uinttype, exponent_max, exponent_min, ieee754_representation, significand_mask +import .Core: AbstractFloat +import .Base: Rational, Float16, Float32, Float64, Bool + using .Base.Libc import ..Rounding: Rounding, rounding_raw, setrounding_raw, rounds_to_nearest, rounds_away_from_zero, tie_breaker_is_to_even, correct_rounding_requires_increment -import ..GMP: ClongMax, CulongMax, CdoubleMax, Limb, libgmp +import ..GMP: ClongMax, CulongMax, CdoubleMax, Limb, libgmp, BigInt import ..FastMath.sincos_fast diff --git a/base/multidimensional.jl b/base/multidimensional.jl index ba08f0679590b..40fff7243cd55 100644 --- a/base/multidimensional.jl +++ b/base/multidimensional.jl @@ -4,10 +4,12 @@ module IteratorsMD import .Base: eltype, length, size, first, last, in, getindex, setindex!, min, max, zero, oneunit, isless, eachindex, - convert, show, iterate, promote_rule, to_indices, copy + convert, show, iterate, promote_rule, to_indices, copy, + isassigned import .Base: +, -, *, (:) import .Base: simd_outer_range, simd_inner_length, simd_index, setindex + import Core: Tuple using .Base: to_index, fill_to_length, tail, safe_tail using .Base: IndexLinear, IndexCartesian, AbstractCartesianIndex, ReshapedArray, ReshapedArrayLF, OneTo, Fix1 diff --git a/base/namedtuple.jl b/base/namedtuple.jl index 991c4d35da52f..f71b13852b953 100644 --- a/base/namedtuple.jl +++ b/base/namedtuple.jl @@ -1,5 +1,7 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license +import Core: NamedTuple + """ NamedTuple diff --git a/base/refpointer.jl b/base/refpointer.jl index 5027462eeb6b6..c5968934aa748 100644 --- a/base/refpointer.jl +++ b/base/refpointer.jl @@ -1,5 +1,7 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license +import Core: Ref + """ Ref{T} diff --git a/base/strings/basic.jl b/base/strings/basic.jl index bf11199143c1e..3f26aed736b8d 100644 --- a/base/strings/basic.jl +++ b/base/strings/basic.jl @@ -1,5 +1,7 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license +import Core: Symbol + """ The `AbstractString` type is the supertype of all string implementations in Julia. Strings are encodings of sequences of [Unicode](https://unicode.org/) diff --git a/base/tuple.jl b/base/tuple.jl index ee3174d783531..2ff8a1185a007 100644 --- a/base/tuple.jl +++ b/base/tuple.jl @@ -1,5 +1,7 @@ # This file is a part of Julia. License is MIT: https://julialang.org/license +import Core: Tuple + # Document NTuple here where we have everything needed for the doc system """ NTuple{N, T} From 9cfb24acd8137a1fc2e84d42f871071ad9f3c0fb Mon Sep 17 00:00:00 2001 From: Ian Butterworth Date: Tue, 18 Feb 2025 07:21:49 -0500 Subject: [PATCH 08/16] Run all `--sysimage-native-code=no` cmdlineargs tests single-threaded (#57445) (cherry picked from commit d43a5ad45a0f351dbacfb01caaa212b3f688a1d4) --- test/cmdlineargs.jl | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/cmdlineargs.jl b/test/cmdlineargs.jl index efcdf52b68093..fb44600886429 100644 --- a/test/cmdlineargs.jl +++ b/test/cmdlineargs.jl @@ -1080,13 +1080,18 @@ run(pipeline(devnull, `$(joinpath(Sys.BINDIR, Base.julia_exename())) --lisp`, de let exename = `$(Base.julia_cmd()) --startup-file=no` @test readchomp(`$exename --sysimage-native-code=yes -E "Bool(Base.JLOptions().use_sysimage_native_code)"`) == "true" - @test readchomp(`$exename --sysimage-native-code=no -E + # TODO: Make this safe in the presence of two single-thread threadpools + # see https://github.com/JuliaLang/julia/issues/57198 + @test readchomp(`$exename --sysimage-native-code=no -t1,0 -E "Bool(Base.JLOptions().use_sysimage_native_code)"`) == "false" end # backtrace contains line number info (esp. on windows #17179) for precomp in ("yes", "no") - succ, out, bt = readchomperrors(`$(Base.julia_cmd()) --startup-file=no --sysimage-native-code=$precomp -E 'sqrt(-2)'`) + # TODO: Make this safe in the presence of two single-thread threadpools + # see https://github.com/JuliaLang/julia/issues/57198 + threads = precomp == "no" ? `-t1,0` : `` + succ, out, bt = readchomperrors(`$(Base.julia_cmd()) $threads --startup-file=no --sysimage-native-code=$precomp -E 'sqrt(-2)'`) @test !succ @test out == "" @test occursin(r"\.jl:(\d+)", bt) From 3c58c106cb6baaf4656941e1dfca90cc967e84c4 Mon Sep 17 00:00:00 2001 From: Valentin Churavy Date: Tue, 18 Feb 2025 13:28:45 +0100 Subject: [PATCH 09/16] Only strip invariant.load from special pointers (#57386) Other backends (in this case NVPTX) require that `invariant.load` metadata is maintained to generate non-coherent loads. Currently, we unconditionally strip that metadata from all loads, since our other uses of it may have become invalid. x-ref: https://github.com/llvm/llvm-project/pull/112834 https://github.com/JuliaGPU/CUDA.jl/issues/2531 --------- Co-authored-by: Gabriel Baraldi (cherry picked from commit 29da86bb983066dd076439c2c7bc5e28dbd611bb) --- src/llvm-late-gc-lowering.cpp | 6 ++++-- test/llvmpasses/late-lower-gc.ll | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/llvm-late-gc-lowering.cpp b/src/llvm-late-gc-lowering.cpp index 1b7551f33ebcd..aef5524c2c4c3 100644 --- a/src/llvm-late-gc-lowering.cpp +++ b/src/llvm-late-gc-lowering.cpp @@ -1979,8 +1979,10 @@ bool LateLowerGCFrame::CleanupIR(Function &F, State *S, bool *CFGModified) { // strip all constant alias information, as it might depend on the gc having // preserved a gc root, which stops being true after this pass (#32215) // similar to RewriteStatepointsForGC::stripNonValidData, but less aggressive - if (I->getMetadata(LLVMContext::MD_invariant_load)) - I->setMetadata(LLVMContext::MD_invariant_load, NULL); + if (auto *LI = dyn_cast(I)){ + if (isSpecialPtr(LI->getPointerOperand()->getType()) && LI->getMetadata(LLVMContext::MD_invariant_load)) + LI->setMetadata(LLVMContext::MD_invariant_load, NULL); + } if (MDNode *TBAA = I->getMetadata(LLVMContext::MD_tbaa)) { if (TBAA->getNumOperands() == 4 && isTBAA(TBAA, {"jtbaa_const", "jtbaa_memoryptr", "jtbaa_memorylen", "tbaa_memoryown"})) { MDNode *MutableTBAA = createMutableTBAAAccessTag(TBAA); diff --git a/test/llvmpasses/late-lower-gc.ll b/test/llvmpasses/late-lower-gc.ll index d294847db8f9d..81a1df61d3bd9 100644 --- a/test/llvmpasses/late-lower-gc.ll +++ b/test/llvmpasses/late-lower-gc.ll @@ -90,6 +90,20 @@ top: ret void } +; Confirm that `invariant.load` on other loads survive +define void @gc_keep_invariant(float addrspace(1)* %0) { +top: +; CHECK-LABEL: @gc_keep_invariant + %pgcstack = call {}*** @julia.get_pgcstack() + %1 = bitcast {}*** %pgcstack to {}** + %current_task = getelementptr inbounds {}*, {}** %1, i64 -12 + +; CHECK: %current_task = getelementptr inbounds ptr, ptr %1, i64 -12 + %2 = load float, ptr addrspace(1) %0, align 4, !invariant.load !1 +; CHECK-NEXT: %2 = load float, ptr addrspace(1) %0, align 4, !invariant.load + ret void +} + define i32 @callee_root({} addrspace(10)* %v0, {} addrspace(10)* %v1) { top: ; CHECK-LABEL: @callee_root From 94d741754b88c7bd3cfb069480d258bd0510ee02 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 19 Feb 2025 10:30:32 -0500 Subject: [PATCH 10/16] Revert "Make emitted egal code more loopy (#54121)" (#57453) This reverts a portion of commit 50833c84d454ef989797e035294ba27b3cca79b7. This algorithm is not able to handle simple cases where there is any internal padding, such as the example of: ``` struct LotsBytes a::Int8 b::NTuple{256,Int} c::Int end ``` Unfortunately fixing it is a bit of a large project right now, so reverting now to fix correctness while working on that. Fixes #55513 (indirectly, by removing broken code) Maybe reopens #54109, although the latency issue it proposes to fix doesn't occur on master even with this revert (just the mediocre looking IR result output returns) (cherry picked from commit a65c2cfb2b9b9e4ea6df9988ad9a54366e6236c7) --- Compiler/test/codegen.jl | 51 -------------- src/codegen.cpp | 141 --------------------------------------- 2 files changed, 192 deletions(-) diff --git a/Compiler/test/codegen.jl b/Compiler/test/codegen.jl index 57a9c26aefac6..4514852a2c0bc 100644 --- a/Compiler/test/codegen.jl +++ b/Compiler/test/codegen.jl @@ -889,57 +889,6 @@ ex54166 = Union{Missing, Int64}[missing -2; missing -2]; dims54166 = (1,2) @test (minimum(ex54166; dims=dims54166)[1] === missing) -# #54109 - Excessive LLVM time for egal -struct DefaultOr54109{T} - x::T - default::Bool -end - -@eval struct Torture1_54109 - $((Expr(:(::), Symbol("x$i"), DefaultOr54109{Float64}) for i = 1:897)...) -end -Torture1_54109() = Torture1_54109((DefaultOr54109(1.0, false) for i = 1:897)...) - -@eval struct Torture2_54109 - $((Expr(:(::), Symbol("x$i"), DefaultOr54109{Float64}) for i = 1:400)...) - $((Expr(:(::), Symbol("x$(i+400)"), DefaultOr54109{Int16}) for i = 1:400)...) -end -Torture2_54109() = Torture2_54109((DefaultOr54109(1.0, false) for i = 1:400)..., (DefaultOr54109(Int16(1), false) for i = 1:400)...) - -@noinline egal_any54109(x, @nospecialize(y::Any)) = x === Base.compilerbarrier(:type, y) - -let ir1 = get_llvm(egal_any54109, Tuple{Torture1_54109, Any}), - ir2 = get_llvm(egal_any54109, Tuple{Torture2_54109, Any}) - - # We can't really do timing on CI, so instead, let's look at the length of - # the optimized IR. The original version had tens of thousands of lines and - # was slower, so just check here that we only have < 500 lines. If somebody, - # implements a better comparison that's larger than that, just re-benchmark - # this and adjust the threshold. - - @test count(==('\n'), ir1) < 500 - @test count(==('\n'), ir2) < 500 -end - -## Regression test for egal of a struct of this size without padding, but with -## non-bitsegal, to make sure that it doesn't accidentally go down the accelerated -## path. -@eval struct BigStructAnyInt - $((Expr(:(::), Symbol("x$i"), Pair{Any, Int}) for i = 1:33)...) -end -BigStructAnyInt() = BigStructAnyInt((Union{Base.inferencebarrier(Float64), Int}=>i for i = 1:33)...) -@test egal_any54109(BigStructAnyInt(), BigStructAnyInt()) - -## For completeness, also test correctness, since we don't have a lot of -## large-struct tests. - -# The two allocations of the same struct will likely have different padding, -# we want to make sure we find them egal anyway - a naive memcmp would -# accidentally look at it. -@test egal_any54109(Torture1_54109(), Torture1_54109()) -@test egal_any54109(Torture2_54109(), Torture2_54109()) -@test !egal_any54109(Torture1_54109(), Torture1_54109((DefaultOr54109(2.0, false) for i = 1:897)...)) - bar54599() = Base.inferencebarrier(true) ? (Base.PkgId(Main),1) : nothing function foo54599() diff --git a/src/codegen.cpp b/src/codegen.cpp index 2f5a12aad326d..b2373ec896cc5 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -3641,61 +3641,6 @@ static Value *emit_bitsunion_compare(jl_codectx_t &ctx, const jl_cgval_t &arg1, return phi; } -struct egal_desc { - size_t offset; - size_t nrepeats; - size_t data_bytes; - size_t padding_bytes; -}; - -template -static size_t emit_masked_bits_compare(callback &emit_desc, jl_datatype_t *aty, egal_desc ¤t_desc) -{ - // Memcmp, but with masked padding - size_t data_bytes = 0; - size_t padding_bytes = 0; - size_t nfields = jl_datatype_nfields(aty); - size_t total_size = jl_datatype_size(aty); - assert(aty->layout->flags.isbitsegal); - for (size_t i = 0; i < nfields; ++i) { - size_t offset = jl_field_offset(aty, i); - size_t fend = i == nfields - 1 ? total_size : jl_field_offset(aty, i + 1); - size_t fsz = jl_field_size(aty, i); - jl_datatype_t *fty = (jl_datatype_t*)jl_field_type(aty, i); - assert(jl_is_datatype(fty)); // union fields should never reach here - assert(fty->layout->flags.isbitsegal); - if (jl_field_isptr(aty, i) || !fty->layout->flags.haspadding) { - // The field has no internal padding - data_bytes += fsz; - if (offset + fsz == fend) { - // The field has no padding after. Merge this into the current - // comparison range and go to next field. - } else { - padding_bytes = fend - offset - fsz; - // Found padding. Either merge this into the current comparison - // range, or emit the old one and start a new one. - if (current_desc.data_bytes == data_bytes && - current_desc.padding_bytes == padding_bytes) { - // Same as the previous range, just note that down, so we - // emit this as a loop. - current_desc.nrepeats += 1; - } else { - if (current_desc.nrepeats != 0) - emit_desc(current_desc); - current_desc.nrepeats = 1; - current_desc.data_bytes = data_bytes; - current_desc.padding_bytes = padding_bytes; - } - data_bytes = 0; - } - } else { - // The field may have internal padding. Recurse this. - data_bytes += emit_masked_bits_compare(emit_desc, fty, current_desc); - } - } - return data_bytes; -} - static Value *emit_bits_compare(jl_codectx_t &ctx, jl_cgval_t arg1, jl_cgval_t arg2) { ++EmittedBitsCompares; @@ -3772,92 +3717,6 @@ static Value *emit_bits_compare(jl_codectx_t &ctx, jl_cgval_t arg1, jl_cgval_t a } return ctx.builder.CreateICmpEQ(answer, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), 0)); } - else if (sz > 512 && jl_struct_try_layout(sty) && sty->layout->flags.isbitsegal) { - Value *varg1 = arg1.inline_roots.empty() && arg1.ispointer() ? data_pointer(ctx, arg1) : - value_to_pointer(ctx, arg1).V; - Value *varg2 = arg2.inline_roots.empty() && arg2.ispointer() ? data_pointer(ctx, arg2) : - value_to_pointer(ctx, arg2).V; - varg1 = emit_pointer_from_objref(ctx, varg1); - varg2 = emit_pointer_from_objref(ctx, varg2); - - // See above for why we want to do this - SmallVector gc_uses; - gc_uses.append(get_gc_roots_for(ctx, arg1)); - gc_uses.append(get_gc_roots_for(ctx, arg2)); - OperandBundleDef OpBundle("jl_roots", gc_uses); - - Value *answer = nullptr; - auto emit_desc = [&](egal_desc desc) { - Value *ptr1 = varg1; - Value *ptr2 = varg2; - if (desc.offset != 0) { - ptr1 = emit_ptrgep(ctx, ptr1, desc.offset); - ptr2 = emit_ptrgep(ctx, ptr2, desc.offset); - } - - Value *new_ptr1 = ptr1; - Value *endptr1 = nullptr; - BasicBlock *postBB = nullptr; - BasicBlock *loopBB = nullptr; - PHINode *answerphi = nullptr; - if (desc.nrepeats != 1) { - // Set up loop - endptr1 = emit_ptrgep(ctx, ptr1, desc.nrepeats * (desc.data_bytes + desc.padding_bytes));; - - BasicBlock *currBB = ctx.builder.GetInsertBlock(); - loopBB = BasicBlock::Create(ctx.builder.getContext(), "egal_loop", ctx.f); - postBB = BasicBlock::Create(ctx.builder.getContext(), "post", ctx.f); - ctx.builder.CreateBr(loopBB); - - ctx.builder.SetInsertPoint(loopBB); - Type *TInt1 = getInt1Ty(ctx.builder.getContext()); - answerphi = ctx.builder.CreatePHI(TInt1, 2); - answerphi->addIncoming(answer ? answer : ConstantInt::get(TInt1, 1), currBB); - answer = answerphi; - - PHINode *itr1 = ctx.builder.CreatePHI(ptr1->getType(), 2); - PHINode *itr2 = ctx.builder.CreatePHI(ptr2->getType(), 2); - - new_ptr1 = emit_ptrgep(ctx, itr1, desc.data_bytes + desc.padding_bytes); - itr1->addIncoming(ptr1, currBB); - itr1->addIncoming(new_ptr1, loopBB); - - Value *new_ptr2 = emit_ptrgep(ctx, itr2, desc.data_bytes + desc.padding_bytes); - itr2->addIncoming(ptr2, currBB); - itr2->addIncoming(new_ptr2, loopBB); - - ptr1 = itr1; - ptr2 = itr2; - } - - // Emit memcmp. TODO: LLVM has a pass to expand this for additional - // performance. - Value *this_answer = ctx.builder.CreateCall(prepare_call(memcmp_func), - { ptr1, - ptr2, - ConstantInt::get(ctx.types().T_size, desc.data_bytes) }, - ArrayRef(&OpBundle, gc_uses.empty() ? 0 : 1)); - this_answer = ctx.builder.CreateICmpEQ(this_answer, ConstantInt::get(getInt32Ty(ctx.builder.getContext()), 0)); - answer = answer ? ctx.builder.CreateAnd(answer, this_answer) : this_answer; - if (endptr1) { - answerphi->addIncoming(answer, loopBB); - Value *loopend = ctx.builder.CreateICmpEQ(new_ptr1, endptr1); - ctx.builder.CreateCondBr(loopend, postBB, loopBB); - ctx.builder.SetInsertPoint(postBB); - } - }; - egal_desc current_desc = {0}; - size_t trailing_data_bytes = emit_masked_bits_compare(emit_desc, sty, current_desc); - assert(current_desc.nrepeats != 0); - emit_desc(current_desc); - if (trailing_data_bytes != 0) { - current_desc.nrepeats = 1; - current_desc.data_bytes = trailing_data_bytes; - current_desc.padding_bytes = 0; - emit_desc(current_desc); - } - return answer; - } else { jl_svec_t *types = sty->types; Value *answer = ConstantInt::get(getInt1Ty(ctx.builder.getContext()), 1); From 015c535a9d00ae76c2259c36545b0bf80cad767d Mon Sep 17 00:00:00 2001 From: Gabriel Baraldi Date: Wed, 19 Feb 2025 12:58:11 -0300 Subject: [PATCH 11/16] Change memory indexing to use the type as index instead of i8 (#57389) also add nsw/nuw flags whenever possible. (cherry picked from commit b9a8d46a234fa0fe625371e107f3ab441a867182) --- src/cgutils.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/cgutils.cpp b/src/cgutils.cpp index 39879503596fe..eeba59aba3aed 100644 --- a/src/cgutils.cpp +++ b/src/cgutils.cpp @@ -4710,10 +4710,9 @@ static jl_cgval_t emit_memoryref(jl_codectx_t &ctx, const jl_cgval_t &ref, jl_cg setName(ctx.emission_context, ovflw, "memoryref_ovflw"); } #endif - boffset = ctx.builder.CreateMul(offset, elsz); - setName(ctx.emission_context, boffset, "memoryref_byteoffset"); - newdata = ctx.builder.CreateGEP(getInt8Ty(ctx.builder.getContext()), data, boffset); - setName(ctx.emission_context, newdata, "memoryref_data_byteoffset"); + Type *elty = isboxed ? ctx.types().T_prjlvalue : julia_type_to_llvm(ctx, jl_tparam1(ref.typ)); + newdata = ctx.builder.CreateGEP(elty, data, offset); + setName(ctx.emission_context, newdata, "memoryref_data_offset"); (void)boffset; // LLVM is very bad at handling GEP with types different from the load if (bc) { BasicBlock *failBB, *endBB; From 9095265d803f624741c0700c87ddec478a2ef256 Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Wed, 19 Feb 2025 16:39:38 -0500 Subject: [PATCH 12/16] using/import: ensure world update after each observable operation (#57467) Fix #57316 (cherry picked from commit a70818c3a4b6e660140c0685e70a8094e095397f) --- src/julia.h | 8 ++++---- src/module.c | 22 ++++++++++----------- src/toplevel.c | 52 ++++++++++++++++++++++++-------------------------- test/worlds.jl | 10 ++++++++++ 4 files changed, 50 insertions(+), 42 deletions(-) diff --git a/src/julia.h b/src/julia.h index 6acd82fdd3401..3206b4f1f033b 100644 --- a/src/julia.h +++ b/src/julia.h @@ -2077,10 +2077,10 @@ JL_DLLEXPORT jl_value_t *jl_checked_assignonce(jl_binding_t *b, jl_module_t *mod JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val(jl_binding_t *b JL_ROOTING_ARGUMENT, jl_module_t *mod, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT JL_MAYBE_UNROOTED); JL_DLLEXPORT jl_binding_partition_t *jl_declare_constant_val2(jl_binding_t *b JL_ROOTING_ARGUMENT, jl_module_t *mod, jl_sym_t *var, jl_value_t *val JL_ROOTED_ARGUMENT JL_MAYBE_UNROOTED, enum jl_partition_kind); JL_DLLEXPORT void jl_module_using(jl_module_t *to, jl_module_t *from); -JL_DLLEXPORT void jl_module_use(jl_module_t *to, jl_module_t *from, jl_sym_t *s); -JL_DLLEXPORT void jl_module_use_as(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname); -JL_DLLEXPORT void jl_module_import(jl_module_t *to, jl_module_t *from, jl_sym_t *s); -JL_DLLEXPORT void jl_module_import_as(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname); +JL_DLLEXPORT void jl_module_use(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s); +JL_DLLEXPORT void jl_module_use_as(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname); +JL_DLLEXPORT void jl_module_import(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s); +JL_DLLEXPORT void jl_module_import_as(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname); JL_DLLEXPORT void jl_module_public(jl_module_t *from, jl_sym_t *s, int exported); JL_DLLEXPORT int jl_is_imported(jl_module_t *m, jl_sym_t *s); JL_DLLEXPORT int jl_module_exports_p(jl_module_t *m, jl_sym_t *var); diff --git a/src/module.c b/src/module.c index ad4613c2d5c56..559bbbe9227c2 100644 --- a/src/module.c +++ b/src/module.c @@ -887,11 +887,11 @@ JL_DLLEXPORT void check_safe_import_from(jl_module_t *m) } // NOTE: we use explici since explicit is a C++ keyword -static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *asname, jl_sym_t *s, int explici) +static void module_import_(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *asname, jl_sym_t *s, int explici) { check_safe_import_from(from); jl_binding_t *b = jl_get_binding(from, s); - jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); + jl_binding_partition_t *bpart = jl_get_binding_partition(b, ct->world_age); if (b->deprecated) { if (jl_get_binding_value(b) == jl_nothing) { // silently skip importing deprecated values assigned to nothing (to allow later mutation) @@ -914,7 +914,7 @@ static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *asname, jl_binding_t *ownerb = b; jl_binding_partition_t *ownerbpart = bpart; - jl_walk_binding_inplace(&ownerb, &ownerbpart, jl_current_task->world_age); + jl_walk_binding_inplace(&ownerb, &ownerbpart, ct->world_age); if (jl_bkind_is_some_guard(jl_binding_kind(ownerbpart))) { jl_printf(JL_STDERR, @@ -964,24 +964,24 @@ static void module_import_(jl_module_t *to, jl_module_t *from, jl_sym_t *asname, JL_UNLOCK(&world_counter_lock); } -JL_DLLEXPORT void jl_module_import(jl_module_t *to, jl_module_t *from, jl_sym_t *s) +JL_DLLEXPORT void jl_module_import(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s) { - module_import_(to, from, s, s, 1); + module_import_(ct, to, from, s, s, 1); } -JL_DLLEXPORT void jl_module_import_as(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname) +JL_DLLEXPORT void jl_module_import_as(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname) { - module_import_(to, from, asname, s, 1); + module_import_(ct, to, from, asname, s, 1); } -JL_DLLEXPORT void jl_module_use(jl_module_t *to, jl_module_t *from, jl_sym_t *s) +JL_DLLEXPORT void jl_module_use(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s) { - module_import_(to, from, s, s, 0); + module_import_(ct, to, from, s, s, 0); } -JL_DLLEXPORT void jl_module_use_as(jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname) +JL_DLLEXPORT void jl_module_use_as(jl_task_t *ct, jl_module_t *to, jl_module_t *from, jl_sym_t *s, jl_sym_t *asname) { - module_import_(to, from, asname, s, 0); + module_import_(ct, to, from, asname, s, 0); } void jl_add_usings_backedge(jl_module_t *from, jl_module_t *to) diff --git a/src/toplevel.c b/src/toplevel.c index dd9adb9bb480f..575f0ac2adbe6 100644 --- a/src/toplevel.c +++ b/src/toplevel.c @@ -488,20 +488,18 @@ static void body_attributes(jl_array_t *body, int *has_ccall, int *has_defs, int } extern size_t jl_require_world; -static jl_module_t *call_require(jl_module_t *mod, jl_sym_t *var) JL_GLOBALLY_ROOTED +static jl_module_t *call_require(jl_task_t *ct, jl_module_t *mod, jl_sym_t *var) JL_GLOBALLY_ROOTED { JL_TIMING(LOAD_IMAGE, LOAD_Require); jl_timing_printf(JL_TIMING_DEFAULT_BLOCK, "%s", jl_symbol_name(var)); int build_mode = jl_options.incremental && jl_generating_output(); jl_module_t *m = NULL; - jl_task_t *ct = jl_current_task; static jl_value_t *require_func = NULL; if (require_func == NULL && jl_base_module != NULL) { require_func = jl_get_global(jl_base_module, jl_symbol("require")); } if (require_func != NULL) { - size_t last_age = ct->world_age; ct->world_age = jl_atomic_load_acquire(&jl_world_counter); if (build_mode && jl_require_world < ct->world_age) ct->world_age = jl_require_world; @@ -510,18 +508,19 @@ static jl_module_t *call_require(jl_module_t *mod, jl_sym_t *var) JL_GLOBALLY_RO reqargs[1] = (jl_value_t*)mod; reqargs[2] = (jl_value_t*)var; m = (jl_module_t*)jl_apply(reqargs, 3); - ct->world_age = last_age; } if (m == NULL || !jl_is_module(m)) { jl_errorf("failed to load module %s", jl_symbol_name(var)); } + ct->world_age = jl_atomic_load_acquire(&jl_world_counter); return m; } // either: // - sets *name and returns the module to import *name from // - sets *name to NULL and returns a module to import -static jl_module_t *eval_import_path(jl_module_t *where, jl_module_t *from JL_PROPAGATES_ROOT, +// also updates world_age +static jl_module_t *eval_import_path(jl_task_t *ct, jl_module_t *where, jl_module_t *from JL_PROPAGATES_ROOT, jl_array_t *args, jl_sym_t **name, const char *keyword) JL_GLOBALLY_ROOTED { if (jl_array_nrows(args) == 0) @@ -546,7 +545,7 @@ static jl_module_t *eval_import_path(jl_module_t *where, jl_module_t *from JL_PR m = jl_base_module; } else { - m = call_require(where, var); + m = call_require(ct, where, var); } if (i == jl_array_nrows(args)) return m; @@ -566,6 +565,8 @@ static jl_module_t *eval_import_path(jl_module_t *where, jl_module_t *from JL_PR } } + ct->world_age = jl_atomic_load_acquire(&jl_world_counter); + while (1) { var = (jl_sym_t*)jl_array_ptr_ref(args, i); if (!jl_is_symbol(var)) @@ -650,17 +651,17 @@ JL_DLLEXPORT jl_method_instance_t *jl_method_instance_for_thunk(jl_code_info_t * return mi; } -static void import_module(jl_module_t *JL_NONNULL m, jl_module_t *import, jl_sym_t *asname) +static void import_module(jl_task_t *ct, jl_module_t *JL_NONNULL m, jl_module_t *import, jl_sym_t *asname) { assert(m); jl_sym_t *name = asname ? asname : import->name; // TODO: this is a bit race-y with what error message we might print jl_binding_t *b = jl_get_module_binding(m, name, 1); - jl_binding_partition_t *bpart = jl_get_binding_partition(b, jl_current_task->world_age); + jl_binding_partition_t *bpart = jl_get_binding_partition(b, ct->world_age); enum jl_partition_kind kind = jl_binding_kind(bpart); if (kind != BINDING_KIND_GUARD && kind != BINDING_KIND_FAILED && kind != BINDING_KIND_DECLARED && kind != BINDING_KIND_IMPLICIT) { // Unlike regular constant declaration, we allow this as long as we eventually end up at a constant. - jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age); + jl_walk_binding_inplace(&b, &bpart, ct->world_age); if (jl_binding_kind(bpart) == BINDING_KIND_CONST || jl_binding_kind(bpart) == BINDING_KIND_BACKDATED_CONST || jl_binding_kind(bpart) == BINDING_KIND_CONST_IMPORT) { // Already declared (e.g. on another thread) or imported. if (bpart->restriction == (jl_value_t*)import) @@ -673,7 +674,7 @@ static void import_module(jl_module_t *JL_NONNULL m, jl_module_t *import, jl_sym } // in `import A.B: x, y, ...`, evaluate the `A.B` part if it exists -static jl_module_t *eval_import_from(jl_module_t *m JL_PROPAGATES_ROOT, jl_expr_t *ex, const char *keyword) +static jl_module_t *eval_import_from(jl_task_t *ct, jl_module_t *m JL_PROPAGATES_ROOT, jl_expr_t *ex, const char *keyword) { if (jl_expr_nargs(ex) == 1 && jl_is_expr(jl_exprarg(ex, 0))) { jl_expr_t *fr = (jl_expr_t*)jl_exprarg(ex, 0); @@ -682,7 +683,7 @@ static jl_module_t *eval_import_from(jl_module_t *m JL_PROPAGATES_ROOT, jl_expr_ jl_expr_t *path = (jl_expr_t*)jl_exprarg(fr, 0); if (((jl_expr_t*)path)->head == jl_dot_sym) { jl_sym_t *name = NULL; - jl_module_t *from = eval_import_path(m, NULL, path->args, &name, keyword); + jl_module_t *from = eval_import_path(ct, m, NULL, path->args, &name, keyword); if (name != NULL) { from = (jl_module_t*)jl_eval_global_var(from, name); if (!from || !jl_is_module(from)) @@ -828,8 +829,7 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val } else if (head == jl_using_sym) { jl_sym_t *name = NULL; - jl_module_t *from = eval_import_from(m, ex, "using"); - ct->world_age = jl_atomic_load_acquire(&jl_world_counter); + jl_module_t *from = eval_import_from(ct, m, ex, "using"); size_t i = 0; if (from) { i = 1; @@ -839,10 +839,10 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val jl_value_t *a = jl_exprarg(ex, i); if (jl_is_expr(a) && ((jl_expr_t*)a)->head == jl_dot_sym) { name = NULL; - jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)a)->args, &name, "using"); + jl_module_t *import = eval_import_path(ct, m, from, ((jl_expr_t*)a)->args, &name, "using"); if (from) { // `using A: B` and `using A: B.c` syntax - jl_module_use(m, import, name); + jl_module_use(ct, m, import, name); } else { jl_module_t *u = import; @@ -857,7 +857,7 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val if (m == jl_main_module && name == NULL) { // TODO: for now, `using A` in Main also creates an explicit binding for `A` // This will possibly be extended to all modules. - import_module(m, u, NULL); + import_module(ct, m, u, NULL); } } continue; @@ -868,12 +868,11 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val if (jl_is_symbol(asname)) { jl_expr_t *path = (jl_expr_t*)jl_exprarg(a, 0); name = NULL; - jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)path)->args, &name, "using"); - ct->world_age = jl_atomic_load_acquire(&jl_world_counter); + jl_module_t *import = eval_import_path(ct, m, from, ((jl_expr_t*)path)->args, &name, "using"); assert(name); check_macro_rename(name, asname, "using"); // `using A: B as C` syntax - jl_module_use_as(m, import, name, asname); + jl_module_use_as(ct, m, import, name, asname); continue; } } @@ -886,8 +885,7 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val } else if (head == jl_import_sym) { jl_sym_t *name = NULL; - jl_module_t *from = eval_import_from(m, ex, "import"); - ct->world_age = jl_atomic_load_acquire(&jl_world_counter); + jl_module_t *from = eval_import_from(ct, m, ex, "import"); size_t i = 0; if (from) { i = 1; @@ -897,14 +895,14 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val jl_value_t *a = jl_exprarg(ex, i); if (jl_is_expr(a) && ((jl_expr_t*)a)->head == jl_dot_sym) { name = NULL; - jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)a)->args, &name, "import"); + jl_module_t *import = eval_import_path(ct, m, from, ((jl_expr_t*)a)->args, &name, "import"); if (name == NULL) { // `import A` syntax - import_module(m, import, NULL); + import_module(ct, m, import, NULL); } else { // `import A.B` or `import A: B` syntax - jl_module_import(m, import, name); + jl_module_import(ct, m, import, name); } continue; } @@ -914,15 +912,15 @@ JL_DLLEXPORT jl_value_t *jl_toplevel_eval_flex(jl_module_t *JL_NONNULL m, jl_val if (jl_is_symbol(asname)) { jl_expr_t *path = (jl_expr_t*)jl_exprarg(a, 0); name = NULL; - jl_module_t *import = eval_import_path(m, from, ((jl_expr_t*)path)->args, &name, "import"); + jl_module_t *import = eval_import_path(ct, m, from, ((jl_expr_t*)path)->args, &name, "import"); if (name == NULL) { // `import A as B` syntax - import_module(m, import, asname); + import_module(ct, m, import, asname); } else { check_macro_rename(name, asname, "import"); // `import A.B as C` syntax - jl_module_import_as(m, import, name, asname); + jl_module_import_as(ct, m, import, name, asname); } continue; } diff --git a/test/worlds.jl b/test/worlds.jl index f4558327c744b..686708c5efd27 100644 --- a/test/worlds.jl +++ b/test/worlds.jl @@ -524,3 +524,13 @@ module AmbigWorldTest convert(Core.Binding, GlobalRef(M2, :x)).partitions.min_world ) end + +module X57316; module Y57316; end; end +module A57316; using ..X57316.Y57316, .Y57316.Y57316; end +module B57316; import ..X57316.Y57316, .Y57316.Y57316; end +module C57316; import ..X57316.Y57316 as Z, .Z.Y57316 as W; end +@test X57316.Y57316 === A57316.Y57316 === B57316.Y57316 === C57316.Z === C57316.W +@test !isdefined(A57316, :X57316) +@test !isdefined(B57316, :X57316) +@test !isdefined(C57316, :X57316) +@test !isdefined(C57316, :Y57316) From 9579515fadf982edc2da9b48eaf7817facd81e0f Mon Sep 17 00:00:00 2001 From: Cody Tapscott <84105208+topolarity@users.noreply.github.com> Date: Wed, 19 Feb 2025 19:27:27 -0500 Subject: [PATCH 13/16] staticdata: Don't use `newm` pointer after it has been invalidated (#57471) The buffer may end up reallocated by the additional writes performed to it in this function. (cherry picked from commit 0fb5fa0276675c54b7dcd3b69e6ec95264f0b2a1) --- src/staticdata.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/staticdata.c b/src/staticdata.c index d69880aa20f71..dc30ed7f64341 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -1344,6 +1344,10 @@ static void jl_write_module(jl_serializer_state *s, uintptr_t item, jl_module_t arraylist_push(&s->relocs_list, (void*)(reloc_offset + offsetof(jl_module_t, usings_backedges))); arraylist_push(&s->relocs_list, (void*)backref_id(s, m->usings_backedges, s->link_ids_relocs)); + // After reload, everything that has happened in this process happened semantically at + // (for .incremental) or before jl_require_world, so reset this flag. + jl_atomic_store_relaxed(&newm->export_set_changed_since_require_world, 0); + // write out the usings list memset(&newm->usings._space, 0, sizeof(newm->usings._space)); if (m->usings.items == &m->usings._space[0]) { @@ -1372,6 +1376,7 @@ static void jl_write_module(jl_serializer_state *s, uintptr_t item, jl_module_t newm->usings.items = (void**)tot; arraylist_push(&s->relocs_list, (void*)(reloc_offset + offsetof(jl_module_t, usings.items))); arraylist_push(&s->relocs_list, (void*)(((uintptr_t)DataRef << RELOC_TAG_OFFSET) + item)); + newm = NULL; // `write_*(s->s)` below may invalidate `newm`, so defensively set it to NULL size_t i; for (i = 0; i < module_usings_length(m); i++) { struct _jl_module_using *data = module_usings_getidx(m, i); @@ -1395,10 +1400,6 @@ static void jl_write_module(jl_serializer_state *s, uintptr_t item, jl_module_t } } assert(ios_pos(s->s) - reloc_offset == tot); - - // After reload, everything that has happened in this process happened semantically at - // (for .incremental) or before jl_require_world, so reset this flag. - jl_atomic_store_relaxed(&newm->export_set_changed_since_require_world, 0); } static void record_memoryref(jl_serializer_state *s, size_t reloc_offset, jl_genericmemoryref_t ref) { From 84c3fd75546b4e784184bfa9642dcffd8755e156 Mon Sep 17 00:00:00 2001 From: Em Chu <61633163+mlechu@users.noreply.github.com> Date: Thu, 20 Feb 2025 14:25:38 -0800 Subject: [PATCH 14/16] lowering: Don't mutate lambda in `linearize` (#57416) Fixes #56904. The associated PR (#55876) compiles a finally block, then compiles a renumbered version of it. This works if `compile` doesn't mutate its input, but in reality, lambda bodies were being `set!` when linearized. The "invalid syntax" error was a result of attempting to linearize a lambda twice. (cherry picked from commit 414aca21e936c3605038eb35a8f58f11067317b4) --- src/julia-syntax.scm | 9 ++++----- src/utils.scm | 13 +++++++++++++ test/syntax.jl | 4 ++++ 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index b6dc7bb9c8a12..47cbc9dec1503 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -4322,11 +4322,10 @@ f(x) = yt(x) (define (linearize e) (cond ((or (not (pair? e)) (quoted? e)) e) ((eq? (car e) 'lambda) - (set-car! (cdddr e) (compile-body (cadddr e) (append (car (caddr e)) - (cadr (caddr e))) - e))) - (else (for-each linearize (cdr e)))) - e) + (list-set e 3 (compile-body (cadddr e) + (append (car (caddr e)) + (cadr (caddr e))) e))) + (else (cons (car e) (map linearize (cdr e)))))) (define (valid-ir-argument? e) (or (simple-atom? e) diff --git a/src/utils.scm b/src/utils.scm index 79e3a280b9886..80fc44615a49a 100644 --- a/src/utils.scm +++ b/src/utils.scm @@ -119,3 +119,16 @@ (cons (car lst) (filter (lambda (x) (not (pred x))) (cdr lst)))) (else (cons (car lst) (keep-first pred (cdr lst)))))) + +(define (take lst n) + (let loop ((lst lst) (n n) (out '())) + (if (= n 0) (reverse out) + (loop (cdr lst) (- n 1) (cons (car lst) out))))) + +(define (drop lst n) + (if (= n 0) lst + (drop (cdr lst) (- n 1)))) + +;; functional update at position i +(define (list-set lst i val) + (append (take lst i) (list val) (drop lst (+ i 1)))) diff --git a/test/syntax.jl b/test/syntax.jl index 366b7f5e63679..13d0d82e20d06 100644 --- a/test/syntax.jl +++ b/test/syntax.jl @@ -4103,3 +4103,7 @@ module Ambig57404 using .B end @test Ambig57404.S == 1 + +# Issue #56904 - lambda linearized twice +@test (let; try 3; finally try 1; f(() -> x); catch x; end; end; x = 7; end) === 7 +@test (let; try 3; finally try 4; finally try 1; f(() -> x); catch x; end; end; end; x = 7; end) === 7 From c78016e4dcd7a202825f0d88323eb899a8dd37f4 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 17 Feb 2025 20:03:38 -0500 Subject: [PATCH 15/16] Don't return null pointer when asking for the type of a declared global (#57447) The `_DECLARED` partition kind used to be considered `guard`, but we now consider it equivalent to an Any-typed `_GLOBAL` (but with weaker redefinition properties). That said, its `->restriction` is NULL, so add it to the list of bindings that should return `nothing` here (and thus `Any` from the bulitin) to fix #57446. (cherry picked from commit 0163991000ab83bfd9f6b5ece695c1e1e6138ea7) --- src/module.c | 5 +++-- test/core.jl | 8 ++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/module.c b/src/module.c index 559bbbe9227c2..848ced7058af1 100644 --- a/src/module.c +++ b/src/module.c @@ -782,9 +782,10 @@ JL_DLLEXPORT jl_value_t *jl_get_binding_type(jl_module_t *m, jl_sym_t *var) if (b == NULL) return jl_nothing; jl_walk_binding_inplace(&b, &bpart, jl_current_task->world_age); - if (jl_bkind_is_some_guard(jl_binding_kind(bpart))) + enum jl_partition_kind kind = jl_binding_kind(bpart); + if (jl_bkind_is_some_guard(kind) || kind == BINDING_KIND_DECLARED) return jl_nothing; - if (jl_bkind_is_some_constant(jl_binding_kind(bpart))) { + if (jl_bkind_is_some_constant(kind)) { // TODO: We would like to return the type of the constant, but // currently code relies on this returning any to bypass conversion // before an attempted assignment to a constant. diff --git a/test/core.jl b/test/core.jl index ef1d6784cac94..4502d82e1f529 100644 --- a/test/core.jl +++ b/test/core.jl @@ -8448,3 +8448,11 @@ myfun57023a(::Type{T}) where {T} = (x = @ccall mycfun()::Ptr{T}; x) @test only(code_lowered(myfun57023a)).has_fcall myfun57023b(::Type{T}) where {T} = (x = @cfunction myfun57023a Ptr{T} (Ref{T},); x) @test only(code_lowered(myfun57023b)).has_fcall + +# issue #57446 +module GlobalAssign57446 + using Test + global theglobal + (@__MODULE__).theglobal = 1 + @test theglobal == 1 +end From bf424632d66f32748361c54bcafb903944ebeec9 Mon Sep 17 00:00:00 2001 From: Kristoffer Date: Tue, 25 Feb 2025 17:24:08 +0100 Subject: [PATCH 16/16] bump LinearAlgebra to latest v1.12 --- .../md5 | 1 - .../sha512 | 1 - .../md5 | 1 + .../sha512 | 1 + stdlib/LinearAlgebra.version | 2 +- 5 files changed, 3 insertions(+), 3 deletions(-) delete mode 100644 deps/checksums/LinearAlgebra-e7da19f2764ba36bd0a9eb8ec67dddce19d87114.tar.gz/md5 delete mode 100644 deps/checksums/LinearAlgebra-e7da19f2764ba36bd0a9eb8ec67dddce19d87114.tar.gz/sha512 create mode 100644 deps/checksums/LinearAlgebra-f0f7a46063ae60a794e3ae6aa2910cfc90b41760.tar.gz/md5 create mode 100644 deps/checksums/LinearAlgebra-f0f7a46063ae60a794e3ae6aa2910cfc90b41760.tar.gz/sha512 diff --git a/deps/checksums/LinearAlgebra-e7da19f2764ba36bd0a9eb8ec67dddce19d87114.tar.gz/md5 b/deps/checksums/LinearAlgebra-e7da19f2764ba36bd0a9eb8ec67dddce19d87114.tar.gz/md5 deleted file mode 100644 index b49f2365717a1..0000000000000 --- a/deps/checksums/LinearAlgebra-e7da19f2764ba36bd0a9eb8ec67dddce19d87114.tar.gz/md5 +++ /dev/null @@ -1 +0,0 @@ -f9258e97e2f478f66a4e63ed008a6953 diff --git a/deps/checksums/LinearAlgebra-e7da19f2764ba36bd0a9eb8ec67dddce19d87114.tar.gz/sha512 b/deps/checksums/LinearAlgebra-e7da19f2764ba36bd0a9eb8ec67dddce19d87114.tar.gz/sha512 deleted file mode 100644 index 4873f3162c8be..0000000000000 --- a/deps/checksums/LinearAlgebra-e7da19f2764ba36bd0a9eb8ec67dddce19d87114.tar.gz/sha512 +++ /dev/null @@ -1 +0,0 @@ -3ed43673c69b8ee549bf5b63d9a6d814f9638269fc0b12d7f7c735581757f1627a3dcd1f242f8fde2cbde1509b43d261191a9a0bb019e22759cb019936ae949e diff --git a/deps/checksums/LinearAlgebra-f0f7a46063ae60a794e3ae6aa2910cfc90b41760.tar.gz/md5 b/deps/checksums/LinearAlgebra-f0f7a46063ae60a794e3ae6aa2910cfc90b41760.tar.gz/md5 new file mode 100644 index 0000000000000..712005f2dcb7f --- /dev/null +++ b/deps/checksums/LinearAlgebra-f0f7a46063ae60a794e3ae6aa2910cfc90b41760.tar.gz/md5 @@ -0,0 +1 @@ +5b6db0a9e4e75f998b63ef9b3f62005c diff --git a/deps/checksums/LinearAlgebra-f0f7a46063ae60a794e3ae6aa2910cfc90b41760.tar.gz/sha512 b/deps/checksums/LinearAlgebra-f0f7a46063ae60a794e3ae6aa2910cfc90b41760.tar.gz/sha512 new file mode 100644 index 0000000000000..42e33bc1a239c --- /dev/null +++ b/deps/checksums/LinearAlgebra-f0f7a46063ae60a794e3ae6aa2910cfc90b41760.tar.gz/sha512 @@ -0,0 +1 @@ +b1bd5744395f692c5902c7074a4e7d3a9cfd7b1905bccdf72b77602ab10d3c1b9a8eb45eac358442da3afc1d26f2e4f4d56f40a0465a70e1d962f0a6a259657e diff --git a/stdlib/LinearAlgebra.version b/stdlib/LinearAlgebra.version index 7b2b896063c56..acc660eb22f59 100644 --- a/stdlib/LinearAlgebra.version +++ b/stdlib/LinearAlgebra.version @@ -1,4 +1,4 @@ LINEARALGEBRA_BRANCH = release-1.12 -LINEARALGEBRA_SHA1 = e7da19f2764ba36bd0a9eb8ec67dddce19d87114 +LINEARALGEBRA_SHA1 = f0f7a46063ae60a794e3ae6aa2910cfc90b41760 LINEARALGEBRA_GIT_URL := https://github.com/JuliaLang/LinearAlgebra.jl.git LINEARALGEBRA_TAR_URL = https://api.github.com/repos/JuliaLang/LinearAlgebra.jl/tarball/$1