Skip to content

Commit

Permalink
[Spesh] Destroy inlinee after inlining
Browse files Browse the repository at this point in the history
After we've inlined a graph the inliner claims ownership over the memory
of the inlinee. With this patch, we destroy the original graph, which
should simplify lifetime management. The memory region list is merged with
the inliners graph so that the memory allocated for the inlinee nodes
is not freed.
  • Loading branch information
bdw committed Sep 3, 2018
1 parent 652056d commit b217a11
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 23 deletions.
12 changes: 12 additions & 0 deletions src/core/regionalloc.c
Expand Up @@ -43,3 +43,15 @@ void MVM_region_destroy(MVMThreadContext *tc, MVMRegionAlloc *alloc) {
}
alloc->block = NULL;
}

/* Link source region into target region, so they can be cleaned up as one */
void MVM_region_merge(MVMThreadContext *tc, MVMRegionAlloc *target, MVMRegionAlloc *source) {
MVMRegionBlock *block = source->block;
while (block != NULL) {
MVMRegionBlock *prev = block->prev;
block->prev = target->block->prev;

This comment has been minimized.

Copy link
@vendethiel

vendethiel Sep 3, 2018

Contributor

target->block is guaranteed non-NULL in this case?

target->block->prev = block;
block = prev;
}
source->block = NULL;
}
1 change: 1 addition & 0 deletions src/core/regionalloc.h
Expand Up @@ -24,3 +24,4 @@ struct MVMRegionAlloc {

void * MVM_region_alloc(MVMThreadContext *tc, MVMRegionAlloc *alloc, size_t s);
void MVM_region_destroy(MVMThreadContext *tc, MVMRegionAlloc *alloc);
void MVM_region_merge(MVMThreadContext *tc, MVMRegionAlloc *target, MVMRegionAlloc *source);
11 changes: 2 additions & 9 deletions src/spesh/candidate.c
Expand Up @@ -137,15 +137,8 @@ void MVM_spesh_candidate_add(MVMThreadContext *tc, MVMSpeshPlanned *p) {
candidate->num_spesh_slots = sg->num_spesh_slots;
candidate->spesh_slots = sg->spesh_slots;

/* Clean up after specialization work. */
if (candidate->num_inlines) {
MVMint32 i;
for (i = 0; i < candidate->num_inlines; i++)
if (candidate->inlines[i].g) {
MVM_spesh_graph_destroy(tc, candidate->inlines[i].g);
candidate->inlines[i].g = NULL;
}
}
/* Claim ownership of allocated memory by candidate */
sg->cand = candidate;
MVM_spesh_graph_destroy(tc, sg);

/* Create a new candidate list and copy any existing ones. Free memory
Expand Down
3 changes: 1 addition & 2 deletions src/spesh/codegen.c
Expand Up @@ -316,9 +316,8 @@ MVMSpeshCode * MVM_spesh_codegen(MVMThreadContext *tc, MVMSpeshGraph *g) {
}
else {
if (g->inlines[i].start == -1 || g->inlines[i].end == -1)
MVM_oops(tc, "Spesh: failed to fix up inline %d %p (%s) %d %d",
MVM_oops(tc, "Spesh: failed to fix up inline %d (%s) %d %d",
i,
g->inlines[i].g,
MVM_string_utf8_maybe_encode_C_string(tc, g->inlines[i].sf->body.name),
g->inlines[i].start,
g->inlines[i].end
Expand Down
19 changes: 15 additions & 4 deletions src/spesh/graph.c
Expand Up @@ -1354,10 +1354,21 @@ void MVM_spesh_graph_mark(MVMThreadContext *tc, MVMSpeshGraph *g, MVMGCWorklist
void MVM_spesh_graph_destroy(MVMThreadContext *tc, MVMSpeshGraph *g) {
/* Free all of the allocated node memory. */
MVM_region_destroy(tc, &g->region_alloc);
/* Free handlers array, if different from the static frame. */
if (!g->cand && g->handlers && g->handlers != g->sf->body.handlers)
MVM_free(g->handlers);

/* If there is a candidate that we either generated or that this graph was
* generated from, it has ownership of the malloc'd memory. If not, then we
* need to clean up */
if (!g->cand) {
/* Free handlers array, if different from the static frame. */
if (g->handlers && g->handlers != g->sf->body.handlers)
MVM_free(g->handlers);
if (g->deopt_addrs)
MVM_free(g->deopt_addrs);
if (g->inlines)
MVM_free(g->inlines);
}
/* When we generate a graph from a candidate, we copy the spesh slots */
if (g->spesh_slots)
MVM_free(g->spesh_slots);
/* Free the graph itself. */
MVM_free(g);
}
11 changes: 8 additions & 3 deletions src/spesh/inline.c
Expand Up @@ -207,7 +207,6 @@ MVMSpeshGraph * MVM_spesh_inline_try_get_graph(MVMThreadContext *tc, MVMSpeshGra
}
else {
/* Not possible to inline. Clear it up. */
MVM_free(ig->spesh_slots);
MVM_spesh_graph_destroy(tc, ig);
return NULL;
}
Expand Down Expand Up @@ -243,7 +242,9 @@ MVMSpeshGraph * MVM_spesh_inline_try_get_graph_from_unspecialized(MVMThreadConte
return ig;
}
else {
MVM_free(ig->spesh_slots);
/* TODO - we spent all this work creating an optimized version. Maybe
we can make use of it by compiling a custom target despite not being
inlineable? */
MVM_spesh_graph_destroy(tc, ig);
return NULL;
}
Expand Down Expand Up @@ -619,7 +620,6 @@ MVMSpeshBB * merge_graph(MVMThreadContext *tc, MVMSpeshGraph *inliner,
}
inliner->inlines[total_inlines - 1].sf = inlinee_sf;
inliner->inlines[total_inlines - 1].code_ref_reg = code_ref_reg.reg.orig;
inliner->inlines[total_inlines - 1].g = inlinee;
inliner->inlines[total_inlines - 1].locals_start = inliner->num_locals;
inliner->inlines[total_inlines - 1].lexicals_start = inliner->num_lexicals;
inliner->inlines[total_inlines - 1].num_locals = inlinee->num_locals;
Expand Down Expand Up @@ -1166,4 +1166,9 @@ void MVM_spesh_inline(MVMThreadContext *tc, MVMSpeshGraph *inliner,
* for the sake of deopt. */
if (inliner->inlines[inliner->num_inlines - 1].may_cause_deopt)
MVM_spesh_usages_retain_deopt_index(tc, inliner, proxy_deopt_idx);

/* Claim ownership of inlinee memory */
MVM_region_merge(tc, &inliner->region_alloc, &inlinee->region_alloc);
/* Destroy the inlinee graph */
MVM_spesh_graph_destroy(tc, inlinee);
}
3 changes: 0 additions & 3 deletions src/spesh/inline.h
Expand Up @@ -39,9 +39,6 @@ struct MVMSpeshInline {
/* Bit field of named args used to put in place during deopt, since we
* typically don't update the array in specialized code. */
MVMuint64 deopt_named_used_bit_field;

/* Inlinee's spesh graph, so we can free it up after code-gen. */
MVMSpeshGraph *g;
};

MVMSpeshGraph * MVM_spesh_inline_try_get_graph(MVMThreadContext *tc,
Expand Down
2 changes: 0 additions & 2 deletions src/spesh/optimize.c
Expand Up @@ -1805,7 +1805,6 @@ static void optimize_call(MVMThreadContext *tc, MVMSpeshGraph *g, MVMSpeshBB *bb
MVM_spesh_usages_add_unconditional_deopt_usage_by_reg(tc, g, code_ref_reg);
MVM_spesh_inline(tc, g, arg_info, bb, ins, inline_graph, target_sf,
code_ref_reg, prepargs_deopt_idx);
MVM_free(inline_graph->spesh_slots);
}
else {
/* Can't inline, so just identify candidate. */
Expand Down Expand Up @@ -1856,7 +1855,6 @@ static void optimize_call(MVMThreadContext *tc, MVMSpeshGraph *g, MVMSpeshBB *bb
MVM_spesh_usages_add_unconditional_deopt_usage_by_reg(tc, g, code_ref_reg);
MVM_spesh_inline(tc, g, arg_info, bb, ins, inline_graph, target_sf,
code_ref_reg, prepargs_deopt_idx);
MVM_free(inline_graph->spesh_slots);
}
}

Expand Down

0 comments on commit b217a11

Please sign in to comment.