Skip to content

Commit

Permalink
Remove unnecessary GC root on the return path
Browse files Browse the repository at this point in the history
Fixes #15457
  • Loading branch information
yuyichao committed Jul 9, 2016
1 parent 2065920 commit 45521d0
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 3 deletions.
6 changes: 5 additions & 1 deletion src/ccall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ class FunctionMover : public ValueMaterializer
// llvmcall(ir, (rettypes...), (argtypes...), args...)
static jl_cgval_t emit_llvmcall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)
{
JL_NARGSV(llvmcall, 3)
JL_NARGSV(llvmcall, 3);
jl_value_t *rt = NULL, *at = NULL, *ir = NULL, *decl = NULL;
jl_svec_t *stt = NULL;
JL_GC_PUSH5(&ir, &rt, &at, &stt, &decl);
Expand Down Expand Up @@ -1007,6 +1007,7 @@ static jl_cgval_t emit_llvmcall(jl_value_t **args, size_t nargs, jl_codectx_t *c
f->setLinkage(GlobalValue::LinkOnceODRLinkage);

// the actual call
builder.CreateCall(prepare_call(gcroot_flush_func));
CallInst *inst = builder.CreateCall(f, ArrayRef<Value*>(&argvals[0], nargt));
if (isString)
ctx->to_inline.push_back(inst);
Expand Down Expand Up @@ -1438,6 +1439,7 @@ static jl_cgval_t emit_ccall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)
assert(!isVa);
assert(nargt == 0);
JL_GC_POP();
builder.CreateCall(prepare_call(gcroot_flush_func));
emit_signal_fence();
builder.CreateLoad(ctx->signalPage, true);
emit_signal_fence();
Expand Down Expand Up @@ -1469,6 +1471,7 @@ static jl_cgval_t emit_ccall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)
assert(!isVa);
assert(nargt == 0);
JL_GC_POP();
builder.CreateCall(prepare_call(gcroot_flush_func));
Value *pdefer_sig = emit_defer_signal(ctx);
Value *defer_sig = builder.CreateLoad(pdefer_sig);
defer_sig = builder.CreateAdd(defer_sig,
Expand All @@ -1484,6 +1487,7 @@ static jl_cgval_t emit_ccall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)
assert(!isVa);
assert(nargt == 0);
JL_GC_POP();
builder.CreateCall(prepare_call(gcroot_flush_func));
Value *pdefer_sig = emit_defer_signal(ctx);
Value *defer_sig = builder.CreateLoad(pdefer_sig);
emit_signal_fence();
Expand Down
8 changes: 8 additions & 0 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,7 @@ static Function *gcroot_func;
static Function *gcstore_func;
static Function *gckill_func;
static Function *jlcall_frame_func;
static Function *gcroot_flush_func;

static std::vector<Type *> two_pvalue_llvmt;
static std::vector<Type *> three_pvalue_llvmt;
Expand Down Expand Up @@ -3461,6 +3462,7 @@ static void finalize_gc_frame(Function *F)
M->getOrInsertFunction(gckill_func->getName(), gckill_func->getFunctionType());
M->getOrInsertFunction(gcstore_func->getName(), gcstore_func->getFunctionType());
M->getOrInsertFunction(jlcall_frame_func->getName(), jlcall_frame_func->getFunctionType());
M->getOrInsertFunction(gcroot_flush_func->getName(), gcroot_flush_func->getFunctionType());
Function *jl_get_ptls_states = M->getFunction("jl_get_ptls_states");

CallInst *ptlsStates = NULL;
Expand Down Expand Up @@ -3531,6 +3533,7 @@ static void finalize_gc_frame(Module *m)
m->getFunction("julia.gc_root_kill")->eraseFromParent();
m->getFunction("julia.gc_store")->eraseFromParent();
m->getFunction("julia.jlcall_frame_decl")->eraseFromParent();
m->getFunction("julia.gcroot_flush")->eraseFromParent();
}

static Function *gen_cfun_wrapper(jl_function_t *ff, jl_value_t *jlrettype, jl_tupletype_t *argt,
Expand Down Expand Up @@ -5659,6 +5662,11 @@ static void init_julia_llvm_env(Module *m)
"julia.jlcall_frame_decl", m);
add_named_global(jlcall_frame_func, (void*)NULL, /*dllimport*/false);

gcroot_flush_func = Function::Create(FunctionType::get(T_void, false),
Function::ExternalLinkage,
"julia.gcroot_flush", m);
add_named_global(gcroot_flush_func, (void*)NULL, /*dllimport*/false);

// set up optimization passes
#ifdef LLVM37
// No DataLayout pass needed anymore.
Expand Down
55 changes: 53 additions & 2 deletions src/llvm-gcroot.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class JuliaGCAllocator {
gckill_func(M.getFunction("julia.gc_root_kill")),
gc_store_func(M.getFunction("julia.gc_store")),
jlcall_frame_func(M.getFunction("julia.jlcall_frame_decl")),
gcroot_flush_func(M.getFunction("julia.gcroot_flush")),
tbaa_gcframe(tbaa)
{
/* Algorithm sketch:
Expand Down Expand Up @@ -82,7 +83,8 @@ Function *const gcroot_func;
Function *const gckill_func;
Function *const gc_store_func;
Function *const jlcall_frame_func;
MDNode *tbaa_gcframe;
Function *const gcroot_flush_func;
MDNode *const tbaa_gcframe;

typedef std::pair<CallInst*, unsigned> frame_register;
class liveness {
Expand Down Expand Up @@ -461,11 +463,59 @@ unsigned find_space_for(CallInst *callInst,
return n;
}

void rearangeRoots()
{
for (auto BB = F.begin(), E(F.end()); BB != E; BB++) {
auto terminst = BB->getTerminator();
if (!dyn_cast<ReturnInst>(terminst) &&
!dyn_cast<UnreachableInst>(terminst))
continue;
SmallVector<StoreInst*, 16> toRemove;
for (auto I = BB->rbegin(), E(BB->rend()); I != E; ++I) {
// Only handle the simplest case for now, give up if there's a call
// or load from the GC frame.
// (Assume we don't have loads that can alias GC frame
// unless the source address is a `julia.gc_root_decl`)
Instruction *inst = &*I;
if (dyn_cast<CallInst>(inst))
break;
if (LoadInst *loadInst = dyn_cast<LoadInst>(inst)) {
if (CallInst *loadAddr =
dyn_cast<CallInst>(loadInst->getPointerOperand())) {
if (loadAddr->getCalledFunction() == gcroot_func) {
break;
}
}
continue;
}
if (StoreInst *storeInst = dyn_cast<StoreInst>(inst)) {
if (CallInst *storeAddr =
dyn_cast<CallInst>(storeInst->getPointerOperand())) {
if (storeAddr->getCalledFunction() == gcroot_func) {
toRemove.push_back(storeInst);
}
}
continue;
}
}
for (auto inst: toRemove) {
CallInst *decl = cast<CallInst>(inst->getPointerOperand());
inst->eraseFromParent();
// TODO removing unused slot should probably be handled later
// when we allocate the frame
if (decl->use_empty()) {
decl->eraseFromParent();
}
}
}
}

public:
void allocate_frame()
{
Instruction *last_gcframe_inst = gcframe;
collapseRedundantRoots();
rearangeRoots();

/* # initialize the kill BasicBlock of all jlcall-frames
* bb-uses : map<BB, map< pair<inst, arg-offset>, assign|live|kill > >
Expand Down Expand Up @@ -665,7 +715,8 @@ void allocate_frame()
++i;
// delete the now unused gckill information
if (CallInst* callInst = dyn_cast<CallInst>(inst)) {
if (callInst->getCalledFunction() == gckill_func) {
Value *callee = callInst->getCalledFunction();
if (callee == gckill_func || callee == gcroot_flush_func) {
callInst->eraseFromParent();
}
}
Expand Down

0 comments on commit 45521d0

Please sign in to comment.