Skip to content

Commit ece1c70

Browse files
authored
Fix codegen not handling invoke exprs with Codeinstances iwith jl_fptr_sparam_addr invoke types. (#56817)
fixes #56739 I didn't succeed in making a test for this. The sole trigger seems to be ```julia using HMMGradients T = Float32 A = T[0.0 1.0 0.0; 0.0 0.5 0.5; 1.0 0.0 0.0] t2tr = Dict{Int,Vector{Int}}[Dict(1 => [2]),] t2IJ= HMMGradients.t2tr2t2IJ(t2tr) Nt = length(t2tr)+1 Ns = size(A,1) y = rand(T,Nt,Ns) c = rand(Float32, Nt) beta = backward(Nt,A,c,t2IJ,y) gamma = posterior(Nt,t2IJ,A,y) ``` in @oscardssmith memorynew PR One other option is to have the builtin handle receiving a CI. That might make the code cleaner and does handle the case where we receive a dynamic CI (is that even a thing)
1 parent 03a0247 commit ece1c70

File tree

2 files changed

+66
-49
lines changed

2 files changed

+66
-49
lines changed

Compiler/test/codegen.jl

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,3 +1027,12 @@ for a in ((@noinline Ref{Int}(2)),
10271027
@test ex === a
10281028
end
10291029
end
1030+
1031+
# Make sure that code that has unbound sparams works
1032+
#https://github.com/JuliaLang/julia/issues/56739
1033+
1034+
f56739(a) where {T} = a
1035+
1036+
@test f56739(1) == 1
1037+
g56739(x) = @noinline f56739(x)
1038+
@test g56739(1) == 1

src/codegen.cpp

Lines changed: 57 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -5484,62 +5484,70 @@ static jl_cgval_t emit_invoke(jl_codectx_t &ctx, const jl_cgval_t &lival, ArrayR
54845484
result = mark_julia_const(ctx, codeinst->rettype_const);
54855485
handled = true;
54865486
}
5487-
else if (invoke != jl_fptr_sparam_addr) {
5487+
else {
54885488
bool specsig, needsparams;
54895489
std::tie(specsig, needsparams) = uses_specsig(mi, codeinst->rettype, ctx.params->prefer_specsig);
5490-
std::string name;
5491-
StringRef protoname;
5492-
bool need_to_emit = true;
5493-
bool cache_valid = ctx.use_cache || ctx.external_linkage;
5494-
bool external = false;
5495-
5496-
// Check if we already queued this up
5497-
auto it = ctx.call_targets.find(codeinst);
5498-
if (need_to_emit && it != ctx.call_targets.end()) {
5499-
assert(it->second.specsig == specsig);
5500-
protoname = it->second.decl->getName();
5501-
need_to_emit = cache_valid = false;
5502-
}
5490+
if (needsparams) {
5491+
if (trim_may_error(ctx.params->trim))
5492+
push_frames(ctx, ctx.linfo, mi);
5493+
Value *r = emit_jlcall(ctx, jlinvoke_func, track_pjlvalue(ctx, literal_pointer_val(ctx, (jl_value_t*)mi)), argv, nargs, julia_call2);
5494+
result = mark_julia_type(ctx, r, true, rt);
5495+
handled = true;
5496+
} else {
5497+
std::string name;
5498+
StringRef protoname;
5499+
bool need_to_emit = true;
5500+
bool cache_valid = ctx.use_cache || ctx.external_linkage;
5501+
bool external = false;
5502+
5503+
// Check if we already queued this up
5504+
auto it = ctx.call_targets.find(codeinst);
5505+
if (need_to_emit && it != ctx.call_targets.end()) {
5506+
assert(it->second.specsig == specsig);
5507+
protoname = it->second.decl->getName();
5508+
need_to_emit = cache_valid = false;
5509+
}
55035510

5504-
// Check if it is already compiled (either JIT or externally)
5505-
if (need_to_emit && cache_valid) {
5506-
// optimization: emit the correct name immediately, if we know it
5507-
// TODO: use `emitted` map here too to try to consolidate names?
5508-
uint8_t specsigflags;
5509-
jl_callptr_t invoke;
5510-
void *fptr;
5511-
jl_read_codeinst_invoke(codeinst, &specsigflags, &invoke, &fptr, 0);
5512-
if (specsig ? specsigflags & 0b1 : invoke == jl_fptr_args_addr) {
5513-
protoname = jl_ExecutionEngine->getFunctionAtAddress((uintptr_t)fptr, invoke, codeinst);
5514-
if (ctx.external_linkage) {
5515-
// TODO: Add !specsig support to aotcompile.cpp
5516-
// Check that the codeinst is containing native code
5517-
if (specsig && (specsigflags & 0b100)) {
5518-
external = true;
5511+
// Check if it is already compiled (either JIT or externally)
5512+
if (need_to_emit && cache_valid) {
5513+
// optimization: emit the correct name immediately, if we know it
5514+
// TODO: use `emitted` map here too to try to consolidate names?
5515+
uint8_t specsigflags;
5516+
jl_callptr_t invoke;
5517+
void *fptr;
5518+
jl_read_codeinst_invoke(codeinst, &specsigflags, &invoke, &fptr, 0);
5519+
if (specsig ? specsigflags & 0b1 : invoke == jl_fptr_args_addr) {
5520+
protoname = jl_ExecutionEngine->getFunctionAtAddress((uintptr_t)fptr, invoke, codeinst);
5521+
if (ctx.external_linkage) {
5522+
// TODO: Add !specsig support to aotcompile.cpp
5523+
// Check that the codeinst is containing native code
5524+
if (specsig && (specsigflags & 0b100)) {
5525+
external = true;
5526+
need_to_emit = false;
5527+
}
5528+
}
5529+
else { // ctx.use_cache
55195530
need_to_emit = false;
55205531
}
55215532
}
5522-
else { // ctx.use_cache
5523-
need_to_emit = false;
5524-
}
55255533
}
5526-
}
5527-
if (need_to_emit) {
5528-
raw_string_ostream(name) << (specsig ? "j_" : "j1_") << name_from_method_instance(mi) << "_" << jl_atomic_fetch_add_relaxed(&globalUniqueGeneratedNames, 1);
5529-
protoname = StringRef(name);
5530-
}
5531-
jl_returninfo_t::CallingConv cc = jl_returninfo_t::CallingConv::Boxed;
5532-
unsigned return_roots = 0;
5533-
if (specsig)
5534-
result = emit_call_specfun_other(ctx, mi, codeinst->rettype, protoname, external ? codeinst : nullptr, argv, nargs, &cc, &return_roots, rt, age_ok);
5535-
else
5536-
result = emit_call_specfun_boxed(ctx, codeinst->rettype, protoname, external ? codeinst : nullptr, argv, nargs, rt, age_ok);
5537-
handled = true;
5538-
if (need_to_emit) {
5539-
Function *trampoline_decl = cast<Function>(jl_Module->getNamedValue(protoname));
5540-
ctx.call_targets[codeinst] = {cc, return_roots, trampoline_decl, nullptr, specsig};
5541-
if (trim_may_error(ctx.params->trim))
5542-
push_frames(ctx, ctx.linfo, mi);
5534+
if (need_to_emit) {
5535+
raw_string_ostream(name) << (specsig ? "j_" : "j1_") << name_from_method_instance(mi) << "_" << jl_atomic_fetch_add_relaxed(&globalUniqueGeneratedNames, 1);
5536+
protoname = StringRef(name);
5537+
}
5538+
jl_returninfo_t::CallingConv cc = jl_returninfo_t::CallingConv::Boxed;
5539+
unsigned return_roots = 0;
5540+
if (specsig)
5541+
result = emit_call_specfun_other(ctx, mi, codeinst->rettype, protoname, external ? codeinst : nullptr, argv, nargs, &cc, &return_roots, rt, age_ok);
5542+
else
5543+
result = emit_call_specfun_boxed(ctx, codeinst->rettype, protoname, external ? codeinst : nullptr, argv, nargs, rt, age_ok);
5544+
handled = true;
5545+
if (need_to_emit) {
5546+
Function *trampoline_decl = cast<Function>(jl_Module->getNamedValue(protoname));
5547+
ctx.call_targets[codeinst] = {cc, return_roots, trampoline_decl, nullptr, specsig};
5548+
if (trim_may_error(ctx.params->trim))
5549+
push_frames(ctx, ctx.linfo, mi);
5550+
}
55435551
}
55445552
}
55455553
}

0 commit comments

Comments
 (0)