Skip to content

Commit dd47fcb

Browse files
vtjnashKristofferC
authored andcommitted
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 (cherry picked from commit 36b046d)
1 parent 1a14429 commit dd47fcb

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)
@@ -3808,7 +3809,7 @@ static void emit_unionmove(jl_codectx_t &ctx, Value *dest, MDNode *tbaa_dst, con
38083809
if (jl_is_pointerfree(typ)) {
38093810
emit_guarded_test(ctx, skip, nullptr, [&] {
38103811
unsigned alignment = julia_alignment(typ);
3811-
emit_unbox_store(ctx, mark_julia_const(ctx, src.constant), dest, tbaa_dst, Align(alignment), isVolatile);
3812+
emit_unbox_store(ctx, mark_julia_const(ctx, src.constant), dest, tbaa_dst, Align(alignment), Align(alignment), isVolatile);
38123813
return nullptr;
38133814
});
38143815
}
@@ -3818,7 +3819,7 @@ static void emit_unionmove(jl_codectx_t &ctx, Value *dest, MDNode *tbaa_dst, con
38183819
if (jl_is_pointerfree(src.typ)) {
38193820
emit_guarded_test(ctx, skip, nullptr, [&] {
38203821
unsigned alignment = julia_alignment(src.typ);
3821-
emit_unbox_store(ctx, src, dest, tbaa_dst, Align(alignment), isVolatile);
3822+
emit_unbox_store(ctx, src, dest, tbaa_dst, Align(alignment), Align(alignment), isVolatile);
38223823
return nullptr;
38233824
});
38243825
}
@@ -4273,6 +4274,8 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
42734274
}
42744275
}
42754276
else {
4277+
Align align_dst(jl_field_align(sty, i));
4278+
Align align_src(julia_alignment(jtype));
42764279
if (field_promotable) {
42774280
fval_info.V->replaceAllUsesWith(dest);
42784281
cast<Instruction>(fval_info.V)->eraseFromParent();
@@ -4281,10 +4284,10 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
42814284
fval = emit_unbox(ctx, fty, fval_info, jtype);
42824285
}
42834286
else if (!roots.empty()) {
4284-
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);
4287+
split_value_into(ctx, fval_info, align_src, dest, align_dst, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), roots);
42854288
}
42864289
else {
4287-
emit_unbox_store(ctx, fval_info, dest, ctx.tbaa().tbaa_stack, Align(jl_field_align(sty, i)));
4290+
emit_unbox_store(ctx, fval_info, dest, ctx.tbaa().tbaa_stack, align_src, align_dst);
42884291
}
42894292
}
42904293
if (init_as_value) {

src/codegen.cpp

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5731,7 +5731,7 @@ static void emit_vi_assignment_unboxed(jl_codectx_t &ctx, jl_varinfo_t &vi, Valu
57315731
if (vi.inline_roots)
57325732
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);
57335733
else
5734-
emit_unbox_store(ctx, rval_info, vi.value.V, tbaa, align, vi.isVolatile);
5734+
emit_unbox_store(ctx, rval_info, vi.value.V, tbaa, align, align, vi.isVolatile);
57355735
}
57365736
}
57375737
}
@@ -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;
@@ -8713,11 +8713,12 @@ static jl_llvm_functions_t
87138713
++AI; // both specsig (derived) and fptr1 (box) pass this argument as a distinct argument
87148714
// Load closure world
87158715
Value *worldaddr = emit_ptrgep(ctx, oc_this, offsetof(jl_opaque_closure_t, world));
8716+
Align alignof_ptr(ctx.types().alignof_ptr);
87168717
jl_cgval_t closure_world = typed_load(ctx, worldaddr, NULL, (jl_value_t*)jl_long_type,
8717-
nullptr, nullptr, false, AtomicOrdering::NotAtomic, false, ctx.types().alignof_ptr.value());
8718+
nullptr, nullptr, false, AtomicOrdering::NotAtomic, false, alignof_ptr.value());
87188719
assert(ctx.world_age_at_entry == nullptr);
87198720
ctx.world_age_at_entry = closure_world.V; // The tls world in a OC is the world of the closure
8720-
emit_unbox_store(ctx, closure_world, world_age_field, ctx.tbaa().tbaa_gcframe, ctx.types().alignof_ptr);
8721+
emit_unbox_store(ctx, closure_world, world_age_field, ctx.tbaa().tbaa_gcframe, alignof_ptr, alignof_ptr);
87218722

87228723
if (s == jl_unused_sym || vi.value.constant)
87238724
continue;
@@ -9475,7 +9476,7 @@ static jl_llvm_functions_t
94759476
if (tracked)
94769477
split_value_into(ctx, typedval, align, dest, align, jl_aliasinfo_t::fromTBAA(ctx, ctx.tbaa().tbaa_stack), mayberoots);
94779478
else
9478-
emit_unbox_store(ctx, typedval, dest, ctx.tbaa().tbaa_stack, align);
9479+
emit_unbox_store(ctx, typedval, dest, ctx.tbaa().tbaa_stack, align, align);
94799480
}
94809481
return mayberoots;
94819482
});
@@ -9510,8 +9511,10 @@ static jl_llvm_functions_t
95109511
else {
95119512
if (VN)
95129513
V = Constant::getNullValue(ctx.types().T_prjlvalue);
9513-
if (dest)
9514-
emit_unbox_store(ctx, val, dest, ctx.tbaa().tbaa_stack, Align(julia_alignment(val.typ)));
9514+
if (dest) {
9515+
Align align(julia_alignment(val.typ));
9516+
emit_unbox_store(ctx, val, dest, ctx.tbaa().tbaa_stack, align, align);
9517+
}
95159518
RTindex = ConstantInt::get(getInt8Ty(ctx.builder.getContext()), tindex);
95169519
}
95179520
}

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)