Skip to content

Commit 59ec75e

Browse files
authored
[backports-release-1.11] Revert "precompile: don't waste memory on useless inferred code" (#57864)
This reverts commit bdf8219, from #56749 **Note that this PR is made against `backports-release-1.11`.** Rationale: when coverage is on, both the native code and the inferred code might be eliminated, a complete loss of all precompilation results. There are intentions to adopt a new strategy for Julia 1.12, but in the meantime we should revert this change since it is "just" a sysimg size reduction. Affected Julia versions: 1.11.3, 1.11.4 xref timholy/SnoopCompile.jl#413 (comment)
1 parent bcbd16e commit 59ec75e

File tree

3 files changed

+6
-34
lines changed

3 files changed

+6
-34
lines changed

base/compiler/effects.jl

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,6 @@ is_inaccessiblemem_or_argmemonly(effects::Effects) = effects.inaccessiblememonly
329329

330330
is_consistent_overlay(effects::Effects) = effects.nonoverlayed === CONSISTENT_OVERLAY
331331

332-
# (sync this with codegen.cpp and staticdata.c effects_foldable functions)
333332
function encode_effects(e::Effects)
334333
return ((e.consistent % UInt32) << 0) |
335334
((e.effect_free % UInt32) << 3) |

src/codegen.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9679,10 +9679,10 @@ jl_llvm_functions_t jl_emit_codeinst(
96799679
// Julia-level optimization will never need to see it
96809680
else if (jl_is_method(def) && // don't delete toplevel code
96819681
inferred != jl_nothing && // and there is something to delete (test this before calling jl_ir_inlining_cost)
9682-
((!effects_foldable(codeinst->ipo_purity_bits) && // don't delete code we may want for irinterp
9683-
(jl_ir_inlining_cost(inferred) == UINT16_MAX) && // don't delete inlineable code
9684-
!jl_generating_output()) || // don't delete code when generating a precompile file, trading memory in the short term for avoiding likely duplicating inference work for aotcompile
9685-
jl_atomic_load_relaxed(&codeinst->invoke) == jl_fptr_const_return_addr)) { // unless it is constant (although this shouldn't have had code in the first place)
9682+
!effects_foldable(codeinst->ipo_purity_bits) && // don't delete code we may want for irinterp
9683+
((jl_ir_inlining_cost(inferred) == UINT16_MAX) || // don't delete inlineable code
9684+
jl_atomic_load_relaxed(&codeinst->invoke) == jl_fptr_const_return_addr) && // unless it is constant
9685+
!(params.imaging_mode || jl_options.incremental)) { // don't delete code when generating a precompile file
96869686
jl_atomic_store_release(&codeinst->inferred, jl_nothing);
96879687
}
96889688
}

src/staticdata.c

Lines changed: 2 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -725,16 +725,6 @@ static uintptr_t jl_fptr_id(void *fptr)
725725
return *(uintptr_t*)pbp;
726726
}
727727

728-
static int effects_foldable(uint32_t effects)
729-
{
730-
// N.B.: This needs to be kept in sync with Core.Compiler.is_foldable(effects, true)
731-
return ((effects & 0x7) == 0) && // is_consistent(effects)
732-
(((effects >> 10) & 0x03) == 0) && // is_noub(effects)
733-
(((effects >> 3) & 0x03) == 0) && // is_effect_free(effects)
734-
((effects >> 6) & 0x01); // is_terminates(effects)
735-
}
736-
737-
738728
// `jl_queue_for_serialization` adds items to `serialization_order`
739729
#define jl_queue_for_serialization(s, v) jl_queue_for_serialization_((s), (jl_value_t*)(v), 1, 0)
740730
static void jl_queue_for_serialization_(jl_serializer_state *s, jl_value_t *v, int recursive, int immediate) JL_GC_DISABLED;
@@ -848,25 +838,8 @@ static void jl_insert_into_serialization_queue(jl_serializer_state *s, jl_value_
848838
// TODO: if (ci in ci->defs->cache)
849839
record_field_change((jl_value_t**)&ci->next, NULL);
850840
}
851-
jl_value_t *inferred = jl_atomic_load_relaxed(&ci->inferred);
852-
if (inferred && inferred != jl_nothing) { // disregard if there is nothing here to delete (e.g. builtins, unspecialized)
853-
if (!is_relocatable_ci(&relocatable_ext_cis, ci))
854-
record_field_change((jl_value_t**)&ci->inferred, jl_nothing);
855-
else if (jl_is_method(ci->def->def.method) && // don't delete toplevel code
856-
ci->def->def.method->source) { // don't delete code from optimized opaque closures that can't be reconstructed (and builtins)
857-
if (jl_atomic_load_relaxed(&ci->max_world) != ~(size_t)0 || // delete all code that cannot run
858-
jl_atomic_load_relaxed(&ci->invoke) == jl_fptr_const_return) { // delete all code that just returns a constant
859-
record_field_change((jl_value_t**)&ci->inferred, jl_nothing);
860-
}
861-
else if (native_functions && // don't delete any code if making a ji file
862-
(ci->owner == jl_nothing) && // don't delete code for external interpreters
863-
!effects_foldable(ci->ipo_purity_bits) && // don't delete code we may want for irinterp
864-
jl_ir_inlining_cost(inferred) == UINT16_MAX) { // don't delete inlineable code
865-
// delete the code now: if we thought it was worth keeping, it would have been converted to object code
866-
record_field_change((jl_value_t**)&ci->inferred, jl_nothing);
867-
}
868-
}
869-
}
841+
if (jl_atomic_load_relaxed(&ci->inferred) && !is_relocatable_ci(&relocatable_ext_cis, ci))
842+
record_field_change((jl_value_t**)&ci->inferred, jl_nothing);
870843
}
871844

872845
if (immediate) // must be things that can be recursively handled, and valid as type parameters

0 commit comments

Comments
 (0)