Skip to content

Commit 1f8932f

Browse files
committed
cleanup emit_unbox issues
always need to call typeassert / update type
1 parent ba1579f commit 1f8932f

File tree

4 files changed

+97
-76
lines changed

4 files changed

+97
-76
lines changed

src/ccall.cpp

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -482,16 +482,33 @@ static const std::string make_errmsg(const char *fname, int n, const char *err)
482482
return msg.str();
483483
}
484484

485+
// bitcast whatever Ptr kind x might be (even if it is part of a union) into Ptr{Cvoid}
486+
// given that the caller already had emit_cpointercheck on this branch, so that
487+
// the conversion is guaranteed to be valid on this runtiem branch
488+
static jl_cgval_t voidpointer_update(jl_codectx_t &ctx, const jl_cgval_t &x)
489+
{
490+
if (x.typ == (jl_value_t*)jl_voidpointer_type)
491+
return x;
492+
if (jl_type_intersection(x.typ, (jl_value_t*)jl_pointer_type) == jl_bottom_type)
493+
return jl_cgval_t();
494+
if (x.constant)
495+
return mark_julia_type(ctx, julia_const_to_llvm(ctx, x.constant), false, jl_voidpointer_type);
496+
if (x.V == nullptr)
497+
return jl_cgval_t();
498+
if (!x.inline_roots.empty() || x.ispointer())
499+
return mark_julia_slot(x.V, (jl_value_t*)jl_voidpointer_type, NULL, x.tbaa);
500+
return mark_julia_type(ctx, x.V, false, jl_voidpointer_type);
501+
}
502+
485503
static jl_cgval_t typeassert_input(jl_codectx_t &ctx, const jl_cgval_t &jvinfo, jl_value_t *jlto, jl_unionall_t *jlto_env, int argn)
486504
{
487505
if (jlto != (jl_value_t*)jl_any_type && !jl_subtype(jvinfo.typ, jlto)) {
488506
if (jlto == (jl_value_t*)jl_voidpointer_type) {
489507
// allow a bit more flexibility for what can be passed to (void*) due to Ref{T} conversion behavior in input
490-
if (!jl_is_cpointer_type(jvinfo.typ)) {
508+
if (!jl_is_cpointer_type(jvinfo.typ))
491509
// emit a typecheck, if not statically known to be correct
492510
emit_cpointercheck(ctx, jvinfo, make_errmsg("ccall", argn + 1, ""));
493-
return update_julia_type(ctx, jvinfo, (jl_value_t*)jl_pointer_type);
494-
}
511+
return voidpointer_update(ctx, jvinfo);
495512
}
496513
else {
497514
// emit a typecheck, if not statically known to be correct
@@ -540,7 +557,7 @@ static Value *julia_to_native(
540557

541558
jvinfo = typeassert_input(ctx, jvinfo, jlto, jlto_env, argn);
542559
if (!byRef)
543-
return emit_unbox(ctx, to, jvinfo, jlto);
560+
return emit_unbox(ctx, to, jvinfo);
544561

545562
// pass the address of an alloca'd thing, not a box
546563
// since those are immutable.
@@ -603,8 +620,8 @@ static void interpret_symbol_arg(jl_codectx_t &ctx, native_sym_arg_t &out, jl_va
603620
const char *errmsg = invalid_symbol_err_msg(ccall);
604621
emit_cpointercheck(ctx, arg1, errmsg);
605622
}
606-
arg1 = update_julia_type(ctx, arg1, (jl_value_t*)jl_voidpointer_type);
607-
jl_ptr = emit_unbox(ctx, ctx.types().T_ptr, arg1, (jl_value_t*)jl_voidpointer_type);
623+
arg1 = voidpointer_update(ctx, arg1);
624+
jl_ptr = emit_unbox(ctx, ctx.types().T_ptr, arg1);
608625
}
609626
else if (jl_is_cpointer_type(jl_typeof(ptr))) {
610627
fptr = *(void(**)(void))jl_data_ptr(ptr);
@@ -1587,8 +1604,11 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
15871604
retval = boxed(ctx, argv[0]);
15881605
retval = emit_pointer_from_objref(ctx, retval /*T_prjlvalue*/);
15891606
}
1607+
else if (tti == (jl_value_t*)jl_voidpointer_type) {
1608+
retval = emit_unbox(ctx, largty, voidpointer_update(ctx, argv[0]));
1609+
}
15901610
else {
1591-
retval = emit_unbox(ctx, largty, argv[0], tti);
1611+
retval = emit_unbox(ctx, largty, update_julia_type(ctx, argv[0], tti));
15921612
}
15931613
// retval is now an untracked jl_value_t*
15941614
if (retboxed)
@@ -1707,7 +1727,7 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
17071727
const int rng_offset = offsetof(jl_tls_states_t, rngseed);
17081728
Value *rng_ptr = ctx.builder.CreateInBoundsGEP(getInt8Ty(ctx.builder.getContext()), ptls_p, ConstantInt::get(ctx.types().T_size, rng_offset / sizeof(int8_t)));
17091729
setName(ctx.emission_context, rng_ptr, "rngseed_ptr");
1710-
Value *val64 = emit_unbox(ctx, getInt64Ty(ctx.builder.getContext()), argv[0], (jl_value_t*)jl_uint64_type);
1730+
Value *val64 = emit_unbox(ctx, getInt64Ty(ctx.builder.getContext()), update_julia_type(ctx, argv[0], (jl_value_t*)jl_uint64_type));
17111731
auto store = ctx.builder.CreateAlignedStore(val64, rng_ptr, Align(sizeof(void*)));
17121732
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe);
17131733
ai.decorateInst(store);
@@ -1881,14 +1901,14 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
18811901
const jl_cgval_t &dst = argv[0];
18821902
const jl_cgval_t &src = argv[1];
18831903
const jl_cgval_t &n = argv[2];
1884-
Value *destp = emit_unbox(ctx, ctx.types().T_ptr, dst, (jl_value_t*)jl_voidpointer_type);
1904+
Value *destp = emit_unbox(ctx, ctx.types().T_ptr, update_julia_type(ctx, dst, (jl_value_t*)jl_voidpointer_type));
18851905

18861906
ctx.builder.CreateMemCpy(
18871907
destp,
18881908
MaybeAlign(1),
1889-
emit_unbox(ctx, ctx.types().T_ptr, src, (jl_value_t*)jl_voidpointer_type),
1909+
emit_unbox(ctx, ctx.types().T_ptr, update_julia_type(ctx, src, (jl_value_t*)jl_voidpointer_type)),
18901910
MaybeAlign(1),
1891-
emit_unbox(ctx, ctx.types().T_size, n, (jl_value_t*)jl_ulong_type),
1911+
emit_unbox(ctx, ctx.types().T_size, update_julia_type(ctx, n, (jl_value_t*)jl_ulong_type)),
18921912
false);
18931913
JL_GC_POP();
18941914
return rt == (jl_value_t*)jl_nothing_type ? ghostValue(ctx, jl_nothing_type) :
@@ -1899,13 +1919,13 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
18991919
const jl_cgval_t &dst = argv[0];
19001920
const jl_cgval_t &val = argv[1];
19011921
const jl_cgval_t &n = argv[2];
1902-
Value *destp = emit_unbox(ctx, ctx.types().T_ptr, dst, (jl_value_t*)jl_voidpointer_type);
1903-
Value *val32 = emit_unbox(ctx, getInt32Ty(ctx.builder.getContext()), val, (jl_value_t*)jl_uint32_type);
1922+
Value *destp = emit_unbox(ctx, ctx.types().T_ptr, update_julia_type(ctx, dst, (jl_value_t*)jl_voidpointer_type));
1923+
Value *val32 = emit_unbox(ctx, getInt32Ty(ctx.builder.getContext()), update_julia_type(ctx, val, (jl_value_t*)jl_int32_type));
19041924
Value *val8 = ctx.builder.CreateTrunc(val32, getInt8Ty(ctx.builder.getContext()), "memset_val");
19051925
ctx.builder.CreateMemSet(
19061926
destp,
19071927
val8,
1908-
emit_unbox(ctx, ctx.types().T_size, n, (jl_value_t*)jl_ulong_type),
1928+
emit_unbox(ctx, ctx.types().T_size, update_julia_type(ctx, n, (jl_value_t*)jl_ulong_type)),
19091929
MaybeAlign(1)
19101930
);
19111931
JL_GC_POP();
@@ -1917,14 +1937,14 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
19171937
const jl_cgval_t &dst = argv[0];
19181938
const jl_cgval_t &src = argv[1];
19191939
const jl_cgval_t &n = argv[2];
1920-
Value *destp = emit_unbox(ctx, ctx.types().T_ptr, dst, (jl_value_t*)jl_voidpointer_type);
1940+
Value *destp = emit_unbox(ctx, ctx.types().T_ptr, update_julia_type(ctx, dst, (jl_value_t*)jl_voidpointer_type));
19211941

19221942
ctx.builder.CreateMemMove(
19231943
destp,
19241944
MaybeAlign(0),
1925-
emit_unbox(ctx, ctx.types().T_ptr, src, (jl_value_t*)jl_voidpointer_type),
1945+
emit_unbox(ctx, ctx.types().T_ptr, update_julia_type(ctx, src, (jl_value_t*)jl_voidpointer_type)),
19261946
MaybeAlign(0),
1927-
emit_unbox(ctx, ctx.types().T_size, n, (jl_value_t*)jl_ulong_type),
1947+
emit_unbox(ctx, ctx.types().T_size, update_julia_type(ctx, n, (jl_value_t*)jl_ulong_type)),
19281948
false);
19291949
JL_GC_POP();
19301950
return rt == (jl_value_t*)jl_nothing_type ? ghostValue(ctx, jl_nothing_type) :
@@ -2018,8 +2038,7 @@ jl_cgval_t function_sig_t::emit_a_ccall(
20182038
if (jl_is_abstract_ref_type(jargty)) {
20192039
if (!jl_is_cpointer_type(arg.typ)) {
20202040
emit_cpointercheck(ctx, arg, "ccall: argument to Ref{T} is not a pointer");
2021-
arg.typ = (jl_value_t*)jl_voidpointer_type;
2022-
arg.isboxed = false;
2041+
arg = voidpointer_update(ctx, arg);
20232042
}
20242043
jargty_in_env = (jl_value_t*)jl_voidpointer_type;
20252044
}

src/cgutils.cpp

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ static Value *emit_pointer_from_objref(jl_codectx_t &ctx, Value *V)
300300
return Call;
301301
}
302302

303-
static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_value_t *jt);
303+
static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x);
304304
static void emit_unbox_store(jl_codectx_t &ctx, const jl_cgval_t &x, Value* dest, MDNode *tbaa_dest, MaybeAlign align_src, Align align_dst, bool isVolatile=false);
305305

306306
static bool type_is_permalloc(jl_value_t *typ)
@@ -378,7 +378,7 @@ static llvm::SmallVector<Value*,0> get_gc_roots_for(jl_codectx_t &ctx, const jl_
378378
else if (jl_is_concrete_immutable(x.typ) && !jl_is_pointerfree(x.typ)) {
379379
jl_value_t *jltype = x.typ;
380380
Type *T = julia_type_to_llvm(ctx, jltype);
381-
Value *agg = emit_unbox(ctx, T, x, jltype);
381+
Value *agg = emit_unbox(ctx, T, x);
382382
SmallVector<unsigned,4> perm_offsets;
383383
find_perm_offsets((jl_datatype_t*)jltype, perm_offsets, 0);
384384
return ExtractTrackedValues(agg, agg->getType(), false, ctx.builder, perm_offsets);
@@ -2380,7 +2380,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
23802380
r = ctx.builder.CreateLoad(realelty, intcast);
23812381
}
23822382
else if (aliasscope || Order != AtomicOrdering::NotAtomic || (tracked_pointers && rhs.inline_roots.empty())) {
2383-
r = emit_unbox(ctx, realelty, rhs, jltype);
2383+
r = emit_unbox(ctx, realelty, rhs);
23842384
}
23852385
if (realelty != elty)
23862386
r = ctx.builder.CreateZExt(r, elty);
@@ -2451,7 +2451,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
24512451
// if this value can be loaded from memory, do that now so that it is sequenced before the atomicmodify
24522452
// and the IR is less dependent on what was emitted before now to create this rhs.
24532453
// Inlining should do okay to clean this up later if there are parts we don't need.
2454-
rhs = jl_cgval_t(emit_unbox(ctx, julia_type_to_llvm(ctx, rhs.typ), rhs, rhs.typ), rhs.typ, NULL);
2454+
rhs = jl_cgval_t(emit_unbox(ctx, julia_type_to_llvm(ctx, rhs.typ), rhs), rhs.typ, NULL);
24552455
}
24562456
bool gcstack_arg = JL_FEAT_TEST(ctx,gcstack_arg);
24572457
Function *op = emit_modifyhelper(ctx, cmpop, *modifyop, jltype, elty, rhs, fname, gcstack_arg);
@@ -2522,7 +2522,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
25222522
Compare = ctx.builder.CreateLoad(realelty, intcast);
25232523
}
25242524
else {
2525-
Compare = emit_unbox(ctx, realelty, cmpop, jltype);
2525+
Compare = emit_unbox(ctx, realelty, cmpop);
25262526
}
25272527
if (realelty != elty)
25282528
Compare = ctx.builder.CreateZExt(Compare, elty);
@@ -2595,7 +2595,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
25952595
ctx.builder.CreateStore(realCompare, intcast);
25962596
}
25972597
else if (Order != AtomicOrdering::NotAtomic || (tracked_pointers && rhs.inline_roots.empty())) {
2598-
r = emit_unbox(ctx, realelty, rhs, jltype);
2598+
r = emit_unbox(ctx, realelty, rhs);
25992599
}
26002600
if (realelty != elty)
26012601
r = ctx.builder.CreateZExt(r, elty);
@@ -2724,7 +2724,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
27242724
else if (!isboxed && intcast_eltyp) {
27252725
assert(issetfield);
27262726
// issetfield doesn't use intcast, so need to reload rhs with the correct type
2727-
r = emit_unbox(ctx, intcast_eltyp, rhs, jltype);
2727+
r = emit_unbox(ctx, intcast_eltyp, rhs);
27282728
}
27292729
if (!isboxed)
27302730
emit_write_multibarrier(ctx, parent, r, rhs.typ);
@@ -3530,7 +3530,7 @@ static Value *call_with_attrs(jl_codectx_t &ctx, JuliaFunction<TypeFn_t> *intr,
35303530
static Value *as_value(jl_codectx_t &ctx, Type *to, const jl_cgval_t &v)
35313531
{
35323532
assert(!v.isboxed);
3533-
return emit_unbox(ctx, to, v, v.typ);
3533+
return emit_unbox(ctx, to, v);
35343534
}
35353535

35363536
static Value *load_i8box(jl_codectx_t &ctx, Value *v, jl_datatype_t *ty)
@@ -4383,7 +4383,7 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
43834383
cast<Instruction>(fval_info.V)->eraseFromParent();
43844384
}
43854385
else if (init_as_value) {
4386-
fval = emit_unbox(ctx, fty, fval_info, jtype);
4386+
fval = emit_unbox(ctx, fty, fval_info);
43874387
}
43884388
else {
43894389
split_value_into(ctx, fval_info, align_src, dest, align_dst, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), false, roots);
@@ -4633,7 +4633,7 @@ static jl_cgval_t emit_memorynew(jl_codectx_t &ctx, jl_datatype_t *typ, jl_cgval
46334633
emptymemBB = BasicBlock::Create(ctx.builder.getContext(), "emptymem");
46344634
nonemptymemBB = BasicBlock::Create(ctx.builder.getContext(), "nonemptymem");
46354635
retvalBB = BasicBlock::Create(ctx.builder.getContext(), "retval");
4636-
auto nel_unboxed = emit_unbox(ctx, ctx.types().T_size, nel, (jl_value_t*)jl_long_type);
4636+
auto nel_unboxed = emit_unbox(ctx, ctx.types().T_size, nel);
46374637
Value *memorynew_empty = ctx.builder.CreateICmpEQ(nel_unboxed, ConstantInt::get(T_size, 0));
46384638
setName(ctx.emission_context, memorynew_empty, "memorynew_empty");
46394639
ctx.builder.CreateCondBr(memorynew_empty, emptymemBB, nonemptymemBB);
@@ -4720,7 +4720,11 @@ static jl_cgval_t emit_memoryref_direct(jl_codectx_t &ctx, const jl_cgval_t &mem
47204720
bool isunion = layout->flags.arrayelem_isunion;
47214721
bool isghost = layout->size == 0;
47224722
Value *boxmem = boxed(ctx, mem);
4723-
Value *i = emit_unbox(ctx, ctx.types().T_size, idx, (jl_value_t*)jl_long_type);
4723+
emit_typecheck(ctx, idx, (jl_value_t*)jl_long_type, "memoryrefnew");
4724+
idx = update_julia_type(ctx, idx, (jl_value_t*)jl_long_type);
4725+
if (idx.typ == jl_bottom_type)
4726+
return jl_cgval_t();
4727+
Value *i = emit_unbox(ctx, ctx.types().T_size, idx);
47244728
Value *idx0 = ctx.builder.CreateSub(i, ConstantInt::get(ctx.types().T_size, 1));
47254729
bool bc = bounds_check_enabled(ctx, inbounds);
47264730
if (bc) {
@@ -4794,7 +4798,7 @@ static jl_cgval_t emit_memoryref(jl_codectx_t &ctx, const jl_cgval_t &ref, jl_cg
47944798
maybeSetName(ctx.emission_context, data, "memoryref_data");
47954799
Value *mem = CreateSimplifiedExtractValue(ctx, V, 1);
47964800
maybeSetName(ctx.emission_context, mem, "memoryref_mem");
4797-
Value *i = emit_unbox(ctx, ctx.types().T_size, idx, (jl_value_t*)jl_long_type);
4801+
Value *i = emit_unbox(ctx, ctx.types().T_size, idx);
47984802
Value *offset = ctx.builder.CreateSub(i, ConstantInt::get(ctx.types().T_size, 1));
47994803
setName(ctx.emission_context, offset, "memoryref_offset");
48004804
Value *elsz = emit_genericmemoryelsize(ctx, mem, ref.typ, false);

0 commit comments

Comments
 (0)