Skip to content

Commit 0e51bdf

Browse files
committed
codegen: remove readonly from abstract type calling convention
Fix some misuses of `jl_is_immutable_datatype`, and rename it to indicate it is not a guarantee of immutable. Refs: #58070 The cfunction code difference isn't normally emitted by codegen since it is just a fallback, but can be observed with various code_llvm type signatures for various combinations of abstract/union/concrete/mutable: f(x::T) where {T} = @cfunction identity Ref{T} (Ref{T},) code_llvm(f, (Any,), dump_module=true, raw=true) code_llvm(f, (Number,), dump_module=true, raw=true) code_llvm(f, (Union{Int32,Int64},), dump_module=true, raw=true) code_llvm(f, (Complex,), dump_module=true, raw=true) code_llvm(f, (Ref{Int},), dump_module=true, raw=true)
1 parent d8e7f99 commit 0e51bdf

File tree

4 files changed

+28
-19
lines changed

4 files changed

+28
-19
lines changed

Compiler/test/codegen.jl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,3 +1033,7 @@ end
10331033
const x57872 = "Hello"
10341034
f57872() = (Core.isdefinedglobal(@__MODULE__, Base.compilerbarrier(:const, :x57872)), x57872) # Extra globalref here to force world age bounds
10351035
@test f57872() == (true, "Hello")
1036+
1037+
@noinline f_mutateany(@nospecialize x) = x[] = 1
1038+
g_mutateany() = (y = Ref(0); f_mutateany(y); y[])
1039+
@test g_mutateany() === 1

src/cgutils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1479,6 +1479,7 @@ static Value *emit_sizeof(jl_codectx_t &ctx, const jl_cgval_t &p)
14791479
return dyn_size;
14801480
}
14811481
}
1482+
*/
14821483

14831484
static Value *emit_datatype_mutabl(jl_codectx_t &ctx, Value *dt)
14841485
{
@@ -1493,7 +1494,6 @@ static Value *emit_datatype_mutabl(jl_codectx_t &ctx, Value *dt)
14931494
mutabl = ctx.builder.CreateLShr(mutabl, 1);
14941495
return ctx.builder.CreateTrunc(mutabl, getInt1Ty(ctx.builder.getContext()));
14951496
}
1496-
*/
14971497

14981498
static Value *emit_datatype_isprimitivetype(jl_codectx_t &ctx, Value *typ)
14991499
{

src/codegen.cpp

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1600,7 +1600,7 @@ static MDNode *best_tbaa(jl_tbaacache_t &tbaa_cache, jl_value_t *jt) {
16001600
// note that this includes jl_isbits, although codegen should work regardless
16011601
static bool jl_is_concrete_immutable(jl_value_t* t)
16021602
{
1603-
return jl_is_immutable_datatype(t) && ((jl_datatype_t*)t)->isconcretetype;
1603+
return jl_may_be_immutable_datatype(t) && ((jl_datatype_t*)t)->isconcretetype;
16041604
}
16051605

16061606
static bool jl_is_pointerfree(jl_value_t* t)
@@ -7385,8 +7385,8 @@ static Function *gen_cfun_wrapper(
73857385
inputarg = mark_julia_type(ctx, val, false, jargty);
73867386
}
73877387
}
7388-
else if (static_at || (!jl_is_typevar(jargty) && !jl_is_immutable_datatype(jargty))) {
7389-
// must be a jl_value_t* (because it's mutable or contains gc roots)
7388+
else if (static_at || (!jl_is_typevar(jargty) && (!jl_is_datatype(jargty) || jl_is_abstracttype(jargty) || jl_is_mutable_datatype(jargty)))) {
7389+
// must be a jl_value_t* (because it is mutable or abstract)
73907390
inputarg = mark_julia_type(ctx, maybe_decay_untracked(ctx, val), true, jargty_proper);
73917391
}
73927392
else {
@@ -7400,31 +7400,36 @@ static Function *gen_cfun_wrapper(
74007400
emit_ptrgep(ctx, nestPtr, jl_array_nrows(*closure_types) * ctx.types().sizeof_ptr),
74017401
Align(sizeof(void*)));
74027402
BasicBlock *boxedBB = BasicBlock::Create(ctx.builder.getContext(), "isboxed", cw);
7403-
BasicBlock *loadBB = BasicBlock::Create(ctx.builder.getContext(), "need-load", cw);
7403+
BasicBlock *notanyBB = BasicBlock::Create(ctx.builder.getContext(), "not-any", cw);
74047404
BasicBlock *unboxedBB = BasicBlock::Create(ctx.builder.getContext(), "maybe-unboxed", cw);
74057405
BasicBlock *isanyBB = BasicBlock::Create(ctx.builder.getContext(), "any", cw);
74067406
BasicBlock *afterBB = BasicBlock::Create(ctx.builder.getContext(), "after", cw);
7407-
Value *isrtboxed = ctx.builder.CreateIsNull(val); // XXX: this is the wrong condition and should be inspecting runtime_dt instead
7408-
ctx.builder.CreateCondBr(isrtboxed, boxedBB, loadBB);
7409-
ctx.builder.SetInsertPoint(boxedBB);
7410-
Value *p1 = val;
7411-
p1 = track_pjlvalue(ctx, p1);
7412-
ctx.builder.CreateBr(afterBB);
7413-
ctx.builder.SetInsertPoint(loadBB);
74147407
Value *isrtany = ctx.builder.CreateICmpEQ(
7415-
literal_pointer_val(ctx, (jl_value_t*)jl_any_type), val);
7416-
ctx.builder.CreateCondBr(isrtany, isanyBB, unboxedBB);
7408+
track_pjlvalue(ctx,literal_pointer_val(ctx, (jl_value_t*)jl_any_type)), runtime_dt);
7409+
ctx.builder.CreateCondBr(isrtany, isanyBB, notanyBB);
74177410
ctx.builder.SetInsertPoint(isanyBB);
7418-
Value *p2 = ctx.builder.CreateAlignedLoad(ctx.types().T_prjlvalue, val, Align(sizeof(void*)));
7411+
Value *p1 = ctx.builder.CreateAlignedLoad(ctx.types().T_prjlvalue, val, Align(sizeof(void*)));
7412+
ctx.builder.CreateBr(afterBB);
7413+
isanyBB = ctx.builder.GetInsertBlock(); // could have changed
7414+
ctx.builder.SetInsertPoint(notanyBB);
7415+
jl_cgval_t runtime_dt_val = mark_julia_type(ctx, runtime_dt, true, jl_any_type);
7416+
Value *isrtboxed = // (!jl_is_datatype(runtime_dt) || !jl_is_concrete_datatype(runtime_dt) || jl_is_mutable_datatype(runtime_dt))
7417+
emit_guarded_test(ctx, emit_exactly_isa(ctx, runtime_dt_val, jl_datatype_type), true, [&] {
7418+
return ctx.builder.CreateOr(ctx.builder.CreateNot(emit_isconcrete(ctx, runtime_dt)), emit_datatype_mutabl(ctx, runtime_dt));
7419+
});
7420+
ctx.builder.CreateCondBr(isrtboxed, boxedBB, unboxedBB);
7421+
ctx.builder.SetInsertPoint(boxedBB);
7422+
Value *p2 = track_pjlvalue(ctx, val);
74197423
ctx.builder.CreateBr(afterBB);
7424+
boxedBB = ctx.builder.GetInsertBlock(); // could have changed
74207425
ctx.builder.SetInsertPoint(unboxedBB);
74217426
Value *p3 = emit_new_bits(ctx, runtime_dt, val);
74227427
unboxedBB = ctx.builder.GetInsertBlock(); // could have changed
74237428
ctx.builder.CreateBr(afterBB);
74247429
ctx.builder.SetInsertPoint(afterBB);
74257430
PHINode *p = ctx.builder.CreatePHI(ctx.types().T_prjlvalue, 3);
7426-
p->addIncoming(p1, boxedBB);
7427-
p->addIncoming(p2, isanyBB);
7431+
p->addIncoming(p1, isanyBB);
7432+
p->addIncoming(p2, boxedBB);
74287433
p->addIncoming(p3, unboxedBB);
74297434
inputarg = mark_julia_type(ctx, p, true, jargty_proper);
74307435
}
@@ -7975,7 +7980,7 @@ static jl_returninfo_t get_specsig_function(jl_codegen_params_t &params, Module
79757980
param.addAttribute(Attribute::ReadOnly);
79767981
ty = PointerType::get(M->getContext(), AddressSpace::Derived);
79777982
}
7978-
else if (isboxed && jl_is_immutable_datatype(jt)) {
7983+
else if (isboxed && jl_may_be_immutable_datatype(jt) && !jl_is_abstracttype(jt)) {
79797984
param.addAttribute(Attribute::ReadOnly);
79807985
}
79817986
else if (jl_is_primitivetype(jt) && ty->isIntegerTy()) {

src/julia.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1620,7 +1620,7 @@ static inline int jl_field_isconst(jl_datatype_t *st, int i) JL_NOTSAFEPOINT
16201620
#define jl_is_mutable(t) (((jl_datatype_t*)t)->name->mutabl)
16211621
#define jl_is_mutable_datatype(t) (jl_is_datatype(t) && (((jl_datatype_t*)t)->name->mutabl))
16221622
#define jl_is_immutable(t) (!((jl_datatype_t*)t)->name->mutabl)
1623-
#define jl_is_immutable_datatype(t) (jl_is_datatype(t) && (!((jl_datatype_t*)t)->name->mutabl))
1623+
#define jl_may_be_immutable_datatype(t) (jl_is_datatype(t) && (!((jl_datatype_t*)t)->name->mutabl))
16241624
#define jl_is_uniontype(v) jl_typetagis(v,jl_uniontype_tag<<4)
16251625
#define jl_is_typevar(v) jl_typetagis(v,jl_tvar_tag<<4)
16261626
#define jl_is_unionall(v) jl_typetagis(v,jl_unionall_tag<<4)

0 commit comments

Comments
 (0)