Skip to content

Commit 18f1c26

Browse files
committed
codegen: mark svec fields GC as release stores
Without a release store, it seems LLVM considers it a data race to have read the initial state on another thread. Marking this as a release store seems sufficient to prevent that optimization. It is also more consistent with how we initialize and write to most other structs, particularly since #55767. Fixes #59547 (more)
1 parent 19ad3be commit 18f1c26

File tree

2 files changed

+26
-6
lines changed

2 files changed

+26
-6
lines changed

src/cgutils.cpp

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2228,6 +2228,9 @@ static jl_cgval_t typed_load(jl_codectx_t &ctx, Value *ptr, Value *idx_0based, j
22282228
}
22292229
Value *instr = nullptr;
22302230
if (!isboxed && jl_is_genericmemoryref_type(jltype)) {
2231+
//We don't specify the stronger expected memory ordering here because of fears it may interfere with vectorization and other optimizations
2232+
//if (Order == AtomicOrdering::NotAtomic)
2233+
// Order = AtomicOrdering::Monotonic;
22312234
// load these FCA as individual fields, so LLVM does not need to split them later
22322235
Value *fld0 = ctx.builder.CreateStructGEP(elty, ptr, 0);
22332236
LoadInst *load0 = ctx.builder.CreateAlignedLoad(elty->getStructElementType(0), fld0, Align(alignment), false);
@@ -2403,11 +2406,26 @@ static jl_cgval_t typed_store(jl_codectx_t &ctx,
24032406
instr = load;
24042407
}
24052408
if (r) {
2406-
StoreInst *store = ctx.builder.CreateAlignedStore(r, ptr, Align(alignment));
2407-
store->setOrdering(Order == AtomicOrdering::NotAtomic && isboxed ? AtomicOrdering::Release : Order);
24082409
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, tbaa);
24092410
ai.noalias = MDNode::concatenate(aliasscope, ai.noalias);
2410-
ai.decorateInst(store);
2411+
if (false && !isboxed && Order == AtomicOrdering::NotAtomic && jl_is_genericmemoryref_type(jltype)) {
2412+
// if enabled, store these FCA as individual fields, so LLVM does not need to split them later and they can use release ordering
2413+
assert(r->getType() == ctx.types().T_jlgenericmemory);
2414+
Value *f1 = ctx.builder.CreateExtractValue(r, 0);
2415+
Value *f2 = ctx.builder.CreateExtractValue(r, 1);
2416+
static_assert(offsetof(jl_genericmemoryref_t, ptr_or_offset) == 0, "wrong field order");
2417+
StoreInst *store = ctx.builder.CreateAlignedStore(f1, ctx.builder.CreateStructGEP(ctx.types().T_jlgenericmemory, ptr, 0), Align(alignment));
2418+
store->setOrdering(AtomicOrdering::Release);
2419+
ai.decorateInst(store);
2420+
store = ctx.builder.CreateAlignedStore(f2, ctx.builder.CreateStructGEP(ctx.types().T_jlgenericmemory, ptr, 1), Align(alignment));
2421+
store->setOrdering(AtomicOrdering::Release);
2422+
ai.decorateInst(store);
2423+
}
2424+
else {
2425+
StoreInst *store = ctx.builder.CreateAlignedStore(r, ptr, Align(alignment));
2426+
store->setOrdering(Order == AtomicOrdering::NotAtomic && isboxed ? AtomicOrdering::Release : Order);
2427+
ai.decorateInst(store);
2428+
}
24112429
}
24122430
else {
24132431
assert(Order == AtomicOrdering::NotAtomic && !isboxed && rhs.typ == jltype);
@@ -4435,10 +4453,11 @@ static jl_cgval_t emit_new_struct(jl_codectx_t &ctx, jl_value_t *ty, size_t narg
44354453
for (size_t i = nargs; i < nf; i++) {
44364454
if (!jl_field_isptr(sty, i) && jl_is_uniontype(jl_field_type(sty, i))) {
44374455
jl_aliasinfo_t ai = jl_aliasinfo_t::fromTBAA(ctx, strctinfo.tbaa);
4438-
ai.decorateInst(ctx.builder.CreateAlignedStore(
4456+
auto *store = ctx.builder.CreateAlignedStore(
44394457
ConstantInt::get(getInt8Ty(ctx.builder.getContext()), 0),
44404458
emit_ptrgep(ctx, strct, jl_field_offset(sty, i) + jl_field_size(sty, i) - 1),
4441-
Align(1)));
4459+
Align(1));
4460+
ai.decorateInst(store);
44424461
}
44434462
}
44444463
// TODO: verify that nargs <= nf (currently handled by front-end)

src/codegen.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4123,7 +4123,8 @@ static bool emit_builtin_call(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f,
41234123
for (size_t i = 0; i < nargs; i++) {
41244124
Value *elem = boxed(ctx, argv[i + 1]);
41254125
Value *elem_ptr = emit_ptrgep(ctx, svec_derived, ctx.types().sizeof_ptr * (i + 1));
4126-
ctx.builder.CreateAlignedStore(elem, elem_ptr, Align(ctx.types().sizeof_ptr));
4126+
auto *store = ctx.builder.CreateAlignedStore(elem, elem_ptr, Align(ctx.types().sizeof_ptr));
4127+
store->setOrdering(AtomicOrdering::Release);
41274128
emit_write_barrier(ctx, svec, elem);
41284129
}
41294130
*ret = mark_julia_type(ctx, svec, true, jl_simplevector_type);

0 commit comments

Comments
 (0)