Skip to content

Commit 36b046d

Browse files
authored
codegen: fix alignment of source in typed_load from a unsafe_load (#57845)
The unsafe_load code tries to reuse some of the logic from safe loads, but needs to be careful to override the parts that are not safe with slower versions of them. Similar to the typo-fix from ec3c02a on v1.11-backports. Seen in the IR for code_llvm(optimize=false, raw=true, unsafe_load, (Ptr{Tuple{UInt128}},)) causing LightBSON to fail. Fix ancapdev/LightBSON.jl#37
1 parent 35e2886 commit 36b046d

File tree

4 files changed

+32
-26
lines changed

4 files changed

+32
-26
lines changed

src/ccall.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -547,7 +547,7 @@ static Value *julia_to_native(
547547
Align align(julia_alignment(jlto));
548548
Value *slot = emit_static_alloca(ctx, to, align);
549549
setName(ctx.emission_context, slot, "native_convert_buffer");
550-
emit_unbox_store(ctx, jvinfo, slot, ctx.tbaa().tbaa_stack, align);
550+
emit_unbox_store(ctx, jvinfo, slot, ctx.tbaa().tbaa_stack, align, align);
551551
return slot;
552552
}
553553

src/cgutils.cpp

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ static Value *emit_pointer_from_objref(jl_codectx_t &ctx, Value *V)
301301
}
302302

303303
static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_value_t *jt);
304-
static void emit_unbox_store(jl_codectx_t &ctx, const jl_cgval_t &x, Value* dest, MDNode *tbaa_dest, Align alignment, bool isVolatile=false);
304+
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)
307307
{
@@ -1090,7 +1090,7 @@ static void split_value_into(jl_codectx_t &ctx, const jl_cgval_t &x, Align align
10901090
return;
10911091
}
10921092
if (inline_roots_ptr == nullptr) {
1093-
emit_unbox_store(ctx, x, dst, ctx.tbaa().tbaa_stack, align_dst, isVolatileStore);
1093+
emit_unbox_store(ctx, x, dst, ctx.tbaa().tbaa_stack, align_src, align_dst, isVolatileStore);
10941094
return;
10951095
}
10961096
Value *src = data_pointer(ctx, value_to_pointer(ctx, x));
@@ -1152,7 +1152,7 @@ static void split_value_into(jl_codectx_t &ctx, const jl_cgval_t &x, Align align
11521152
return;
11531153
}
11541154
if (inline_roots.empty()) {
1155-
emit_unbox_store(ctx, x, dst, ctx.tbaa().tbaa_stack, align_dst);
1155+
emit_unbox_store(ctx, x, dst, ctx.tbaa().tbaa_stack, align_src, align_dst, false);
11561156
return;
11571157
}
11581158
Value *src = data_pointer(ctx, value_to_pointer(ctx, x));
@@ -2351,7 +2351,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
23512351
r = boxed(ctx, rhs);
23522352
}
23532353
else if (intcast) {
2354-
emit_unbox_store(ctx, rhs, intcast, ctx.tbaa().tbaa_stack, intcast->getAlign());
2354+
emit_unbox_store(ctx, rhs, intcast, ctx.tbaa().tbaa_stack, MaybeAlign(), intcast->getAlign());
23552355
r = ctx.builder.CreateLoad(realelty, intcast);
23562356
}
23572357
else if (aliasscope || Order != AtomicOrdering::NotAtomic || (tracked_pointers && rhs.inline_roots.empty())) {
@@ -2389,7 +2389,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
23892389
}
23902390
else {
23912391
assert(Order == AtomicOrdering::NotAtomic && !isboxed && rhs.typ == jltype);
2392-
emit_unbox_store(ctx, rhs, ptr, tbaa, Align(alignment));
2392+
emit_unbox_store(ctx, rhs, ptr, tbaa, MaybeAlign(), Align(alignment));
23932393
}
23942394
}
23952395
else if (isswapfield) {
@@ -2438,7 +2438,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
24382438
}
24392439
cmp = update_julia_type(ctx, cmp, jltype);
24402440
if (intcast) {
2441-
emit_unbox_store(ctx, cmp, intcast, ctx.tbaa().tbaa_stack, intcast->getAlign());
2441+
emit_unbox_store(ctx, cmp, intcast, ctx.tbaa().tbaa_stack, MaybeAlign(), intcast->getAlign());
24422442
Compare = ctx.builder.CreateLoad(realelty, intcast);
24432443
}
24442444
else {
@@ -2509,7 +2509,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
25092509
r = boxed(ctx, rhs);
25102510
}
25112511
else if (intcast) {
2512-
emit_unbox_store(ctx, rhs, intcast, ctx.tbaa().tbaa_stack, intcast->getAlign());
2512+
emit_unbox_store(ctx, rhs, intcast, ctx.tbaa().tbaa_stack, MaybeAlign(), intcast->getAlign());
25132513
r = ctx.builder.CreateLoad(realelty, intcast);
25142514
if (!tracked_pointers) // oldval is a slot, so put the oldval back
25152515
ctx.builder.CreateStore(realCompare, intcast);
@@ -2556,7 +2556,7 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
25562556
}
25572557
else {
25582558
assert(!isboxed && rhs.typ == jltype);
2559-
emit_unbox_store(ctx, rhs, ptr, tbaa, Align(alignment));
2559+
emit_unbox_store(ctx, rhs, ptr, tbaa, MaybeAlign(), Align(alignment));
25602560
}
25612561
ctx.builder.CreateBr(DoneBB);
25622562
instr = load;
@@ -3352,9 +3352,10 @@ static void init_bits_value(jl_codectx_t &ctx, Value *newv, Value *v, MDNode *tb
33523352
static void init_bits_cgval(jl_codectx_t &ctx, Value *newv, const jl_cgval_t &v)
33533353
{
33543354
MDNode *tbaa = jl_is_mutable(v.typ) ? ctx.tbaa().tbaa_mutab : ctx.tbaa().tbaa_immut;
3355-
Align newv_align{std::max(julia_alignment(v.typ), (unsigned)sizeof(void*))};
3355+
unsigned alignment = julia_alignment(v.typ);
3356+
unsigned newv_align = std::max(alignment, (unsigned)sizeof(void*));
33563357
newv = maybe_decay_tracked(ctx, newv);
3357-
emit_unbox_store(ctx, v, newv, tbaa, newv_align);
3358+
emit_unbox_store(ctx, v, newv, tbaa, Align(alignment), Align(newv_align));
33583359
}
33593360

33603361
static jl_value_t *static_constant_instance(const llvm::DataLayout &DL, Constant *constant, jl_value_t *jt)
@@ -3816,7 +3817,7 @@ static void emit_unionmove(jl_codectx_t &ctx, Value *dest, MDNode *tbaa_dst, con
38163817
if (jl_is_pointerfree(typ)) {
38173818
emit_guarded_test(ctx, skip, nullptr, [&] {
38183819
unsigned alignment = julia_alignment(typ);
3819-
emit_unbox_store(ctx, mark_julia_const(ctx, src.constant), dest, tbaa_dst, Align(alignment), isVolatile);
3820+
emit_unbox_store(ctx, mark_julia_const(ctx, src.constant), dest, tbaa_dst, Align(alignment), Align(alignment), isVolatile);
38203821
return nullptr;
38213822
});
38223823
}
@@ -3826,7 +3827,7 @@ static void emit_unionmove(jl_codectx_t &ctx, Value *dest, MDNode *tbaa_dst, con
38263827
if (jl_is_pointerfree(src.typ)) {
38273828
emit_guarded_test(ctx, skip, nullptr, [&] {
38283829
unsigned alignment = julia_alignment(src.typ);
3829-
emit_unbox_store(ctx, src, dest, tbaa_dst, Align(alignment), isVolatile);
3830+
emit_unbox_store(ctx, src, dest, tbaa_dst, Align(alignment), Align(alignment), isVolatile);
38303831
return nullptr;
38313832
});
38323833
}
@@ -4289,6 +4290,8 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
42894290
}
42904291
}
42914292
else {
4293+
Align align_dst(jl_field_align(sty, i));
4294+
Align align_src(julia_alignment(jtype));
42924295
if (field_promotable) {
42934296
fval_info.V->replaceAllUsesWith(dest);
42944297
cast<Instruction>(fval_info.V)->eraseFromParent();
@@ -4297,10 +4300,10 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
42974300
fval = emit_unbox(ctx, fty, fval_info, jtype);
42984301
}
42994302
else if (!roots.empty()) {
4300-
split_value_into(ctx, fval_info, Align(julia_alignment(jtype)), dest, Align(jl_field_align(sty, i)), jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), roots);
4303+
split_value_into(ctx, fval_info, align_src, dest, align_dst, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), roots);
43014304
}
43024305
else {
4303-
emit_unbox_store(ctx, fval_info, dest, ctx.tbaa().tbaa_stack, Align(jl_field_align(sty, i)));
4306+
emit_unbox_store(ctx, fval_info, dest, ctx.tbaa().tbaa_stack, align_src, align_dst);
43044307
}
43054308
}
43064309
if (init_as_value) {

src/codegen.cpp

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5724,7 +5724,7 @@ static void emit_vi_assignment_unboxed(jl_codectx_t &ctx, jl_varinfo_t &vi, Valu
57245724
if (vi.inline_roots)
57255725
split_value_into(ctx, rval_info, align, vi.value.V, align, jl_aliasinfo_t::fromTBAA(ctx, tbaa), vi.inline_roots, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe), vi.isVolatile);
57265726
else
5727-
emit_unbox_store(ctx, rval_info, vi.value.V, tbaa, align, vi.isVolatile);
5727+
emit_unbox_store(ctx, rval_info, vi.value.V, tbaa, align, align, vi.isVolatile);
57285728
}
57295729
}
57305730
}
@@ -6947,7 +6947,7 @@ static void emit_specsig_to_specsig(
69476947
split_value_into(ctx, gf_retval, align, sret, align, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), roots, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_gcframe));
69486948
}
69496949
else {
6950-
emit_unbox_store(ctx, gf_retval, sret, ctx.tbaa().tbaa_stack, align);
6950+
emit_unbox_store(ctx, gf_retval, sret, ctx.tbaa().tbaa_stack, align, align);
69516951
}
69526952
ctx.builder.CreateRetVoid();
69536953
break;
@@ -8730,11 +8730,12 @@ static jl_llvm_functions_t
87308730
++AI; // both specsig (derived) and fptr1 (box) pass this argument as a distinct argument
87318731
// Load closure world
87328732
Value *worldaddr = emit_ptrgep(ctx, oc_this, offsetof(jl_opaque_closure_t, world));
8733+
Align alignof_ptr(ctx.types().alignof_ptr);
87338734
jl_cgval_t closure_world = typed_load(ctx, worldaddr, NULL, (jl_value_t*)jl_long_type,
8734-
nullptr, nullptr, false, AtomicOrdering::NotAtomic, false, ctx.types().alignof_ptr.value());
8735+
nullptr, nullptr, false, AtomicOrdering::NotAtomic, false, alignof_ptr.value());
87358736
assert(ctx.world_age_at_entry == nullptr);
87368737
ctx.world_age_at_entry = closure_world.V; // The tls world in a OC is the world of the closure
8737-
emit_unbox_store(ctx, closure_world, world_age_field, ctx.tbaa().tbaa_gcframe, ctx.types().alignof_ptr);
8738+
emit_unbox_store(ctx, closure_world, world_age_field, ctx.tbaa().tbaa_gcframe, alignof_ptr, alignof_ptr);
87388739

87398740
if (s == jl_unused_sym || vi.value.constant)
87408741
continue;
@@ -9502,7 +9503,7 @@ static jl_llvm_functions_t
95029503
if (tracked)
95039504
split_value_into(ctx, typedval, align, dest, align, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), mayberoots);
95049505
else
9505-
emit_unbox_store(ctx, typedval, dest, ctx.tbaa().tbaa_stack, align);
9506+
emit_unbox_store(ctx, typedval, dest, ctx.tbaa().tbaa_stack, align, align);
95069507
}
95079508
return mayberoots;
95089509
});
@@ -9537,8 +9538,10 @@ static jl_llvm_functions_t
95379538
else {
95389539
if (VN)
95399540
V = Constant::getNullValue(ctx.types().T_prjlvalue);
9540-
if (dest)
9541-
emit_unbox_store(ctx, val, dest, ctx.tbaa().tbaa_stack, Align(julia_alignment(val.typ)));
9541+
if (dest) {
9542+
Align align(julia_alignment(val.typ));
9543+
emit_unbox_store(ctx, val, dest, ctx.tbaa().tbaa_stack, align, align);
9544+
}
95429545
RTindex = ConstantInt::get(getInt8Ty(ctx.builder.getContext()), tindex);
95439546
}
95449547
}

src/intrinsics.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ static Value *emit_unbox(jl_codectx_t &ctx, Type *to, const jl_cgval_t &x, jl_va
495495
}
496496

497497
// emit code to store a raw value into a destination
498-
static void emit_unbox_store(jl_codectx_t &ctx, const jl_cgval_t &x, Value *dest, MDNode *tbaa_dest, Align alignment, bool isVolatile)
498+
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)
499499
{
500500
if (x.isghost) {
501501
// this can happen when a branch yielding a different type ends
@@ -507,22 +507,22 @@ static void emit_unbox_store(jl_codectx_t &ctx, const jl_cgval_t &x, Value *dest
507507
auto dest_ai = jl_aliasinfo_t::fromTBAA(ctx, tbaa_dest);
508508

509509
if (!x.inline_roots.empty()) {
510-
recombine_value(ctx, x, dest, dest_ai, alignment, isVolatile);
510+
recombine_value(ctx, x, dest, dest_ai, align_dst, isVolatile);
511511
return;
512512
}
513513

514514
if (!x.ispointer()) { // already unboxed, but sometimes need conversion (e.g. f32 -> i32)
515515
assert(x.V);
516516
Value *unboxed = zext_struct(ctx, x.V);
517-
StoreInst *store = ctx.builder.CreateAlignedStore(unboxed, dest, alignment);
517+
StoreInst *store = ctx.builder.CreateAlignedStore(unboxed, dest, align_dst);
518518
store->setVolatile(isVolatile);
519519
dest_ai.decorateInst(store);
520520
return;
521521
}
522522

523523
Value *src = data_pointer(ctx, x);
524524
auto src_ai = jl_aliasinfo_t::fromTBAA(ctx, x.tbaa);
525-
emit_memcpy(ctx, dest, dest_ai, src, src_ai, jl_datatype_size(x.typ), Align(alignment), Align(julia_alignment(x.typ)), isVolatile);
525+
emit_memcpy(ctx, dest, dest_ai, src, src_ai, jl_datatype_size(x.typ), Align(align_dst), align_src ? *align_src : Align(julia_alignment(x.typ)), isVolatile);
526526
}
527527

528528
static jl_datatype_t *staticeval_bitstype(const jl_cgval_t &targ)

0 commit comments

Comments
 (0)