Skip to content

Commit 775bdc0

Browse files
vtjnashsimeonschaub
authored andcommitted
threading: update codegen to use atomic annotations also
And add load/store alignment annotations, because LLVM now prefers that we try to specify those explicitly, even though it's not required. This does not yet include correct load/store behaviors for objects with inlined references (the recent JuliaLang#34126 PR).
1 parent 475933c commit 775bdc0

12 files changed

+242
-178
lines changed

src/atomics.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@
7575
// TODO: Maybe add jl_atomic_compare_exchange_weak for spin lock
7676
# define jl_atomic_store(obj, val) \
7777
__atomic_store_n(obj, val, __ATOMIC_SEQ_CST)
78-
# define jl_atomic_store_relaxed(obj, val) \
78+
# define jl_atomic_store_relaxed(obj, val) \
7979
__atomic_store_n(obj, val, __ATOMIC_RELAXED)
8080
# if defined(__clang__) || defined(__ICC) || defined(__INTEL_COMPILER) || \
8181
!(defined(_CPU_X86_) || defined(_CPU_X86_64_))
@@ -271,6 +271,7 @@ static inline void jl_atomic_store_release(volatile T *obj, T2 val)
271271
jl_signal_fence();
272272
*obj = (T)val;
273273
}
274+
template<typename T, typename T2>
274275
static inline void jl_atomic_store_relaxed(volatile T *obj, T2 val)
275276
{
276277
*obj = (T)val;

src/ccall.cpp

+20-17
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,15 @@ static Value *runtime_sym_lookup(
8787
BasicBlock *dlsym_lookup = BasicBlock::Create(jl_LLVMContext, "dlsym");
8888
BasicBlock *ccall_bb = BasicBlock::Create(jl_LLVMContext, "ccall");
8989
Constant *initnul = ConstantPointerNull::get((PointerType*)T_pvoidfunc);
90-
LoadInst *llvmf_orig = irbuilder.CreateAlignedLoad(llvmgv, sizeof(void*));
90+
LoadInst *llvmf_orig = irbuilder.CreateAlignedLoad(T_pvoidfunc, llvmgv, sizeof(void*));
9191
// This in principle needs a consume ordering so that load from
9292
// this pointer sees a valid value. However, this is not supported by
9393
// LLVM (or agreed on in the C/C++ standard FWIW) and should be
9494
// almost impossible to happen on every platform we support since this
9595
// ordering is enforced by the hardware and LLVM has to speculate an
9696
// invalid load from the `cglobal` but doesn't depend on the `cglobal`
9797
// value for this to happen.
98-
// llvmf_orig->setAtomic(AtomicOrdering::Consume);
98+
llvmf_orig->setAtomic(AtomicOrdering::Unordered);
9999
irbuilder.CreateCondBr(
100100
irbuilder.CreateICmpNE(llvmf_orig, initnul),
101101
ccall_bb,
@@ -114,7 +114,7 @@ static Value *runtime_sym_lookup(
114114
}
115115
Value *llvmf = irbuilder.CreateCall(prepare_call_in(jl_builderModule(irbuilder), jldlsym_func),
116116
{ libname, stringConstPtr(emission_context, irbuilder, f_name), libptrgv });
117-
auto store = irbuilder.CreateAlignedStore(llvmf, llvmgv, sizeof(void*));
117+
StoreInst *store = irbuilder.CreateAlignedStore(llvmf, llvmgv, sizeof(void*));
118118
store->setAtomic(AtomicOrdering::Release);
119119
irbuilder.CreateBr(ccall_bb);
120120

@@ -169,7 +169,7 @@ static GlobalVariable *emit_plt_thunk(
169169
IRBuilder<> irbuilder(b0);
170170
Value *ptr = runtime_sym_lookup(emission_context, irbuilder, funcptype, f_lib, f_name, plt, libptrgv,
171171
llvmgv, runtime_lib);
172-
auto store = irbuilder.CreateAlignedStore(irbuilder.CreateBitCast(ptr, T_pvoidfunc), got, sizeof(void*));
172+
StoreInst *store = irbuilder.CreateAlignedStore(irbuilder.CreateBitCast(ptr, T_pvoidfunc), got, sizeof(void*));
173173
store->setAtomic(AtomicOrdering::Release);
174174
SmallVector<Value*, 16> args;
175175
for (Function::arg_iterator arg = plt->arg_begin(), arg_e = plt->arg_end(); arg != arg_e; ++arg)
@@ -234,7 +234,7 @@ static Value *emit_plt(
234234
// consume ordering too. This is even less likely to cause issues though
235235
// since the only thing we do to this loaded pointer is to call it
236236
// immediately.
237-
// got_val->setAtomic(AtomicOrdering::Consume);
237+
got_val->setAtomic(AtomicOrdering::Unordered);
238238
return ctx.builder.CreateBitCast(got_val, funcptype);
239239
}
240240

@@ -349,17 +349,19 @@ static Value *llvm_type_rewrite(
349349
Value *from;
350350
Value *to;
351351
const DataLayout &DL = jl_data_layout;
352+
unsigned align = std::max(DL.getPrefTypeAlignment(target_type), DL.getPrefTypeAlignment(from_type));
352353
if (DL.getTypeAllocSize(target_type) >= DL.getTypeAllocSize(from_type)) {
353354
to = emit_static_alloca(ctx, target_type);
355+
cast<AllocaInst>(to)->setAlignment(Align(align));
354356
from = emit_bitcast(ctx, to, from_type->getPointerTo());
355357
}
356358
else {
357359
from = emit_static_alloca(ctx, from_type);
360+
cast<AllocaInst>(from)->setAlignment(Align(align));
358361
to = emit_bitcast(ctx, from, target_type->getPointerTo());
359362
}
360-
// XXX: deal with possible alignment issues
361-
ctx.builder.CreateStore(v, from);
362-
return ctx.builder.CreateLoad(to);
363+
ctx.builder.CreateAlignedStore(v, from, align);
364+
return ctx.builder.CreateAlignedLoad(to, align);
363365
}
364366

365367
// --- argument passing and scratch space utilities ---
@@ -1576,9 +1578,9 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
15761578
Value *ptls_i16 = emit_bitcast(ctx, ctx.ptlsStates, T_pint16);
15771579
const int tid_offset = offsetof(jl_tls_states_t, tid);
15781580
Value *ptid = ctx.builder.CreateGEP(ptls_i16, ConstantInt::get(T_size, tid_offset / 2));
1579-
return mark_or_box_ccall_result(ctx,
1580-
tbaa_decorate(tbaa_const, ctx.builder.CreateLoad(ptid)),
1581-
retboxed, rt, unionall, static_rt);
1581+
LoadInst *tid = ctx.builder.CreateAlignedLoad(ptid, sizeof(int16_t));
1582+
tbaa_decorate(tbaa_const, tid);
1583+
return mark_or_box_ccall_result(ctx, tid, retboxed, rt, unionall, static_rt);
15821584
}
15831585
else if (is_libjulia_func(jl_get_current_task)) {
15841586
assert(lrt == T_prjlvalue);
@@ -1587,9 +1589,9 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
15871589
Value *ptls_pv = emit_bitcast(ctx, ctx.ptlsStates, T_pprjlvalue);
15881590
const int ct_offset = offsetof(jl_tls_states_t, current_task);
15891591
Value *pct = ctx.builder.CreateGEP(ptls_pv, ConstantInt::get(T_size, ct_offset / sizeof(void*)));
1590-
return mark_or_box_ccall_result(ctx,
1591-
tbaa_decorate(tbaa_const, ctx.builder.CreateLoad(pct)),
1592-
retboxed, rt, unionall, static_rt);
1592+
LoadInst *ct = ctx.builder.CreateAlignedLoad(pct, sizeof(void*));
1593+
tbaa_decorate(tbaa_const, ct);
1594+
return mark_or_box_ccall_result(ctx, ct, retboxed, rt, unionall, static_rt);
15931595
}
15941596
else if (is_libjulia_func(jl_set_next_task)) {
15951597
assert(lrt == T_void);
@@ -1608,8 +1610,7 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
16081610
ctx.builder.CreateCall(prepare_call(gcroot_flush_func));
16091611
Value *pdefer_sig = emit_defer_signal(ctx);
16101612
Value *defer_sig = ctx.builder.CreateLoad(pdefer_sig);
1611-
defer_sig = ctx.builder.CreateAdd(defer_sig,
1612-
ConstantInt::get(T_sigatomic, 1));
1613+
defer_sig = ctx.builder.CreateAdd(defer_sig, ConstantInt::get(T_sigatomic, 1));
16131614
ctx.builder.CreateStore(defer_sig, pdefer_sig);
16141615
emit_signal_fence(ctx);
16151616
return ghostValue(jl_nothing_type);
@@ -1671,7 +1672,9 @@ static jl_cgval_t emit_ccall(jl_codectx_t &ctx, jl_value_t **args, size_t nargs)
16711672
idx = ctx.builder.CreateAdd(idx, ConstantInt::get(T_size, ((jl_datatype_t*)ety)->layout->first_ptr));
16721673
}
16731674
Value *slot_addr = ctx.builder.CreateInBoundsGEP(T_prjlvalue, arrayptr, idx);
1674-
Value *load = tbaa_decorate(tbaa_ptrarraybuf, ctx.builder.CreateLoad(T_prjlvalue, slot_addr));
1675+
LoadInst *load = ctx.builder.CreateAlignedLoad(T_prjlvalue, slot_addr, sizeof(void*));
1676+
load->setAtomic(AtomicOrdering::Unordered);
1677+
tbaa_decorate(tbaa_ptrarraybuf, load);
16751678
Value *res = ctx.builder.CreateZExt(ctx.builder.CreateICmpNE(load, Constant::getNullValue(T_prjlvalue)), T_int32);
16761679
JL_GC_POP();
16771680
return mark_or_box_ccall_result(ctx, res, retboxed, rt, unionall, static_rt);

0 commit comments

Comments
 (0)