Skip to content

Commit 21c5f51

Browse files
topolaritynilesh646
topolarity
authored and
nilesh646
committed
cfunction: store fptr / world contiguously (JuliaLang#58684)
1 parent 7859701 commit 21c5f51

File tree

5 files changed

+30
-35
lines changed

5 files changed

+30
-35
lines changed

‎src/aotcompile.cpp‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,7 @@ static void generate_cfunc_thunks(jl_codegen_params_t &params, jl_compiled_funct
576576
}
577577
size_t latestworld = jl_atomic_load_acquire(&jl_world_counter);
578578
for (cfunc_decl_t &cfunc : params.cfuncs) {
579-
Module *M = cfunc.theFptr->getParent();
579+
Module *M = cfunc.cfuncdata->getParent();
580580
jl_value_t *sigt = cfunc.sigt;
581581
JL_GC_PROMISE_ROOTED(sigt);
582582
jl_value_t *declrt = cfunc.declrt;
@@ -585,19 +585,20 @@ static void generate_cfunc_thunks(jl_codegen_params_t &params, jl_compiled_funct
585585
jl_code_instance_t *codeinst = nullptr;
586586
auto assign_fptr = [&params, &cfunc, &codeinst, &unspec](Function *f) {
587587
ConstantArray *init = cast<ConstantArray>(cfunc.cfuncdata->getInitializer());
588-
SmallVector<Constant*,6> initvals;
588+
SmallVector<Constant*,8> initvals;
589589
for (unsigned i = 0; i < init->getNumOperands(); ++i)
590590
initvals.push_back(init->getOperand(i));
591-
assert(initvals.size() == 6);
591+
assert(initvals.size() == 8);
592592
assert(initvals[0]->isNullValue());
593+
assert(initvals[2]->isNullValue());
593594
if (codeinst) {
594595
Constant *llvmcodeinst = literal_pointer_val_slot(params, f->getParent(), (jl_value_t*)codeinst);
595-
initvals[0] = llvmcodeinst; // plast_codeinst
596+
initvals[2] = llvmcodeinst; // plast_codeinst
596597
}
597-
assert(initvals[2]->isNullValue());
598-
initvals[2] = unspec;
598+
assert(initvals[4]->isNullValue());
599+
initvals[4] = unspec;
600+
initvals[0] = f;
599601
cfunc.cfuncdata->setInitializer(ConstantArray::get(init->getType(), initvals));
600-
cfunc.theFptr->setInitializer(f);
601602
};
602603
Module *defM = nullptr;
603604
StringRef func;

‎src/codegen.cpp‎

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,8 +1368,8 @@ static const auto jlgetabiconverter_func = new JuliaFunction<TypeFnContextAndSiz
13681368
XSTR(jl_get_abi_converter),
13691369
[](LLVMContext &C, Type *T_size) {
13701370
Type *T_ptr = getPointerTy(C);
1371-
return FunctionType::get(T_ptr,
1372-
{T_ptr, T_ptr, T_ptr, T_ptr}, false); },
1371+
return FunctionType::get(T_ptr, {T_ptr, T_ptr}, false);
1372+
},
13731373
nullptr,
13741374
};
13751375
static const auto diff_gc_total_bytes_func = new JuliaFunction<>{
@@ -7167,26 +7167,23 @@ static jl_cgval_t emit_abi_call(jl_codectx_t &ctx, jl_value_t *declrt, jl_value_
71677167
Type *T_size = ctx.types().T_size;
71687168
Constant *Vnull = ConstantPointerNull::get(T_ptr);
71697169
Module *M = jl_Module;
7170-
GlobalVariable *theFptr = new GlobalVariable(*M, T_ptr, false,
7171-
GlobalVariable::PrivateLinkage,
7172-
Vnull);
7173-
GlobalVariable *last_world_p = new GlobalVariable(*M, T_size, false,
7174-
GlobalVariable::PrivateLinkage,
7175-
ConstantInt::get(T_size, 0));
7176-
ArrayType *T_cfuncdata = ArrayType::get(T_ptr, 6);
7170+
ArrayType *T_cfuncdata = ArrayType::get(T_ptr, 8);
71777171
size_t flags = specsig;
71787172
GlobalVariable *cfuncdata = new GlobalVariable(*M, T_cfuncdata, false,
71797173
GlobalVariable::PrivateLinkage,
71807174
ConstantArray::get(T_cfuncdata, {
7175+
Vnull,
7176+
Vnull,
71817177
Vnull,
71827178
Vnull,
71837179
Vnull,
71847180
literal_pointer_val_slot(ctx.emission_context, M, declrt),
71857181
literal_pointer_val_slot(ctx.emission_context, M, sigt),
71867182
literal_static_pointer_val((void*)flags, T_ptr)}));
7183+
Value *last_world_p = ctx.builder.CreateConstInBoundsGEP1_32(ctx.types().T_size, cfuncdata, 1);
71877184
LoadInst *last_world_v = ctx.builder.CreateAlignedLoad(T_size, last_world_p, ctx.types().alignof_ptr);
71887185
last_world_v->setOrdering(AtomicOrdering::Acquire);
7189-
LoadInst *callee = ctx.builder.CreateAlignedLoad(T_ptr, theFptr, ctx.types().alignof_ptr);
7186+
LoadInst *callee = ctx.builder.CreateAlignedLoad(T_ptr, cfuncdata, ctx.types().alignof_ptr);
71907187
callee->setOrdering(AtomicOrdering::Monotonic);
71917188
LoadInst *world_v = ctx.builder.CreateAlignedLoad(ctx.types().T_size,
71927189
prepare_global_in(M, jlgetworld_global), ctx.types().alignof_ptr);
@@ -7195,15 +7192,11 @@ static jl_cgval_t emit_abi_call(jl_codectx_t &ctx, jl_value_t *declrt, jl_value_
71957192
Value *age_not_ok = ctx.builder.CreateICmpNE(last_world_v, world_v);
71967193
Value *target = emit_guarded_test(ctx, age_not_ok, callee, [&] {
71977194
Function *getcaller = prepare_call(jlgetabiconverter_func);
7198-
CallInst *cw = ctx.builder.CreateCall(getcaller, {
7199-
get_current_task(ctx),
7200-
theFptr,
7201-
last_world_p,
7202-
cfuncdata});
7195+
CallInst *cw = ctx.builder.CreateCall(getcaller, {get_current_task(ctx), cfuncdata});
72037196
cw->setAttributes(getcaller->getAttributes());
72047197
return cw;
72057198
});
7206-
ctx.emission_context.cfuncs.push_back({declrt, sigt, nargs, specsig, theFptr, cfuncdata});
7199+
ctx.emission_context.cfuncs.push_back({declrt, sigt, nargs, specsig, cfuncdata});
72077200
if (specsig) {
72087201
// TODO: could we force this to guarantee passing a box for `f` here (since we
72097202
// know we had it here) and on the receiver end (emit_abi_converter /

‎src/jitlayers.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -224,7 +224,6 @@ struct cfunc_decl_t {
224224
jl_value_t *sigt;
225225
size_t nargs;
226226
bool specsig;
227-
llvm::GlobalVariable *theFptr;
228227
llvm::GlobalVariable *cfuncdata;
229228
};
230229

‎src/julia_internal.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1628,7 +1628,7 @@ JL_DLLEXPORT jl_value_t *jl_get_cfunction_trampoline(
16281628
jl_value_t *fobj, jl_datatype_t *result, htable_t *cache, jl_svec_t *fill,
16291629
void *(*init_trampoline)(void *tramp, void **nval),
16301630
jl_unionall_t *env, jl_value_t **vals);
1631-
JL_DLLEXPORT void *jl_get_abi_converter(jl_task_t *ct, _Atomic(void*) *fptr, _Atomic(size_t) *last_world, void *data);
1631+
JL_DLLEXPORT void *jl_get_abi_converter(jl_task_t *ct, void *data);
16321632
JL_DLLIMPORT void *jl_jit_abi_converter(jl_task_t *ct, void *unspecialized, jl_value_t *declrt, jl_value_t *sigt, size_t nargs, int specsig,
16331633
jl_code_instance_t *codeinst, jl_callptr_t invoke, void *target, int target_specsig);
16341634

‎src/runtime_ccall.cpp‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,8 @@ jl_value_t *jl_get_cfunction_trampoline(
327327
JL_GCC_IGNORE_STOP
328328

329329
struct cfuncdata_t {
330+
_Atomic(void *) fptr;
331+
_Atomic(size_t) last_world;
330332
jl_code_instance_t** plast_codeinst;
331333
jl_code_instance_t* last_codeinst;
332334
void *unspecialized;
@@ -358,7 +360,7 @@ static jl_mutex_t cfun_lock;
358360
// read theFptr
359361
// acquire jl_world_counter
360362
extern "C" JL_DLLEXPORT
361-
void *jl_get_abi_converter(jl_task_t *ct, _Atomic(void*) *fptr, _Atomic(size_t) *last_world, void *data)
363+
void *jl_get_abi_converter(jl_task_t *ct, void *data)
362364
{
363365
cfuncdata_t *cfuncdata = (cfuncdata_t*)data;
364366
jl_value_t *sigt = *cfuncdata->sigt;
@@ -373,8 +375,8 @@ void *jl_get_abi_converter(jl_task_t *ct, _Atomic(void*) *fptr, _Atomic(size_t)
373375
// check first, while behind this lock, of the validity of the current contents of this cfunc thunk
374376
JL_LOCK(&cfun_lock);
375377
do {
376-
size_t last_world_v = jl_atomic_load_relaxed(last_world);
377-
void *f = jl_atomic_load_relaxed(fptr);
378+
size_t last_world_v = jl_atomic_load_relaxed(&cfuncdata->last_world);
379+
void *f = jl_atomic_load_relaxed(&cfuncdata->fptr);
378380
jl_code_instance_t *last_ci = cfuncdata->plast_codeinst ? *cfuncdata->plast_codeinst : nullptr;
379381
world = jl_atomic_load_acquire(&jl_world_counter);
380382
ct->world_age = world;
@@ -386,14 +388,14 @@ void *jl_get_abi_converter(jl_task_t *ct, _Atomic(void*) *fptr, _Atomic(size_t)
386388
if (f != nullptr) {
387389
if (last_ci == nullptr) {
388390
if (mi == nullptr) {
389-
jl_atomic_store_release(last_world, world);
391+
jl_atomic_store_release(&cfuncdata->last_world, world);
390392
JL_UNLOCK(&cfun_lock);
391393
return f;
392394
}
393395
}
394396
else {
395397
if (jl_get_ci_mi(last_ci) == mi && jl_atomic_load_relaxed(&last_ci->max_world) >= world) { // same dispatch and source
396-
jl_atomic_store_release(last_world, world);
398+
jl_atomic_store_release(&cfuncdata->last_world, world);
397399
JL_UNLOCK(&cfun_lock);
398400
return f;
399401
}
@@ -406,17 +408,17 @@ void *jl_get_abi_converter(jl_task_t *ct, _Atomic(void*) *fptr, _Atomic(size_t)
406408
JL_LOCK(&cfun_lock);
407409
} while (jl_atomic_load_acquire(&jl_world_counter) != world); // restart entirely, since jl_world_counter changed thus jl_get_specialization1 might have changed
408410
// double-check if the values were set on another thread
409-
size_t last_world_v = jl_atomic_load_relaxed(last_world);
410-
void *f = jl_atomic_load_relaxed(fptr);
411+
size_t last_world_v = jl_atomic_load_relaxed(&cfuncdata->last_world);
412+
void *f = jl_atomic_load_relaxed(&cfuncdata->fptr);
411413
if (world == last_world_v) {
412414
JL_UNLOCK(&cfun_lock);
413415
return f; // another thread fixed this up while we were away
414416
}
415-
auto assign_fptr = [fptr, last_world, cfuncdata, world, codeinst](void *f) {
417+
auto assign_fptr = [cfuncdata, world, codeinst](void *f) {
416418
cfuncdata->plast_codeinst = &cfuncdata->last_codeinst;
417419
cfuncdata->last_codeinst = codeinst;
418-
jl_atomic_store_relaxed(fptr, f);
419-
jl_atomic_store_release(last_world, world);
420+
jl_atomic_store_relaxed(&cfuncdata->fptr, f);
421+
jl_atomic_store_release(&cfuncdata->last_world, world);
420422
JL_UNLOCK(&cfun_lock);
421423
return f;
422424
};

0 commit comments

Comments
 (0)