Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DataType not contained in roots of Method #44344

Closed
petvana opened this issue Feb 25, 2022 · 1 comment
Closed

DataType not contained in roots of Method #44344

petvana opened this issue Feb 25, 2022 · 1 comment

Comments

@petvana
Copy link
Member

petvana commented Feb 25, 2022

While working on PR #44332 I found that there is a problem that struct X is not in the roots of setindex! Method. So, I've prepared MWE to demonstrate the issue. Not sure, if this is a bug or not. (The failing CI test comes originally from #43793.)

using Test

mutable struct Dict2{K,V} <: AbstractDict{K,V}
    pairs::Vector{Pair{K,V}} # stored pairs (key::K => value::V)

    function Dict2{K,V}() where V where K
        n = 16
        new(Vector{Pair{K,V}}(undef, n))
    end
end

function setindex2!(h::Dict2{K,Any}, v, key::K) where K
    @nospecialize v
    h.pairs[1] = Pair{K,Any}(key, v)   
end

struct X end
@noinline function f(d)
    @noinline
    setindex2!(d, nothing, X())
end
function callboth()
    f(Dict2{X,Any}())
    nothing
end
precompile(callboth, ())

m = which(setindex2!, (Dict2{X,Any}, Any, X))
@test X in m.roots

Outputs:

Test Failed at REPL[9]:1
  Expression: X in m.roots
   Evaluated: X in Any[:(:pairs), :setindex2!, Symbol("REPL[3]"), Pair{X, Any}, :($(QuoteNode(X()))), :($(QuoteNode(Pair{X, Any}(X(), nothing))))]
ERROR: There was an error during testinwe.jl:29
@timholy
Copy link
Member

timholy commented Feb 25, 2022

FWIW, that test just exploits Dict as a convenient example of code where compilation creates new roots. It would be fine to change that test to something else that does the same thing.

Codegen root-creation is in

julia/src/codegen.cpp

Lines 2368 to 2389 in 9b8253c

static void jl_add_method_root(jl_codectx_t &ctx, jl_value_t *val)
{
if (jl_is_concrete_type(val) || jl_is_bool(val) || jl_is_symbol(val) || val == jl_nothing ||
val == (jl_value_t*)jl_any_type || val == (jl_value_t*)jl_bottom_type || val == (jl_value_t*)jl_core_module)
return;
JL_GC_PUSH1(&val);
if (ctx.roots == NULL) {
ctx.roots = jl_alloc_vec_any(1);
jl_array_ptr_set(ctx.roots, 0, val);
}
else {
size_t rlen = jl_array_dim0(ctx.roots);
for (size_t i = 0; i < rlen; i++) {
if (jl_array_ptr_ref(ctx.roots,i) == val) {
JL_GC_POP();
return;
}
}
jl_array_ptr_1d_push(ctx.roots, val);
}
JL_GC_POP();
}

and compression root-generation is in
jl_add_method_root(s->method, jl_precompile_toplevel_module, v);

which calls

julia/src/method.c

Lines 1049 to 1063 in 9b8253c

JL_DLLEXPORT void jl_add_method_root(jl_method_t *m, jl_module_t *mod, jl_value_t* root)
{
JL_GC_PUSH2(&m, &root);
uint64_t modid = 0;
if (mod) {
assert(jl_is_module(mod));
modid = mod->build_id;
}
assert(jl_is_method(m));
prepare_method_for_roots(m, modid);
if (current_root_id(m->root_blocks) != modid)
add_root_block(m->root_blocks, modid, jl_array_len(m->roots));
jl_array_ptr_1d_push(m->roots, root);
JL_GC_POP();
}

@petvana petvana closed this as completed Feb 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants