Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions src/passes/Heap2Local.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,12 @@ struct Heap2LocalOptimizer {
continue;
}

convertToLocals(allocation);
// Check for escaping, noting relevant information as we go. If this does
// not escape, optimize it.
Rewriter rewriter(allocation, func, module);
if (!escapes(allocation, rewriter)) {
rewriter.applyOptimization();
}
}
}

Expand Down Expand Up @@ -518,11 +523,10 @@ struct Heap2LocalOptimizer {
Mixes,
};

// Analyze an allocation to see if we can convert it from a heap allocation to
// locals.
void convertToLocals(StructNew* allocation) {
Rewriter rewriter(allocation, func, module);

// Analyze an allocation to see if it escapes or not. We receive a Rewriter
// instance on which we store important information as we go, which will be
// necessary if we optimize later.
bool escapes(StructNew* allocation, Rewriter& rewriter) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's odd that the analysis depends on the rewriter. Would it be much work to factor out the parts of the rewriter that the analysis uses as well?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's more a matter of efficiency. To do this in a single pass versus two passes, it makes sense to check for escaping while we note what we need to fix up if we do find nothing escapes.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I don't mean that we should do this in more passes, but it would be nice to move rewriter.sets and rewriter.reaches out of the rewriter and into the analyzer instead.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, interesting. Yeah, maybe a larger refactoring is worthwhile here. I'll look into that and update this PR with my findings.

// A queue of flows from children to parents. When something is in the queue
// here then it assumed that it is ok for the allocation to be at the child
// (that is, we have already checked the child before placing it in the
Expand All @@ -546,7 +550,7 @@ struct Heap2LocalOptimizer {
interaction == ParentChildInteraction::Mixes) {
// If the parent may let us escape, or the parent mixes other values
// up with us, give up.
return;
return true;
}

// The parent either fully consumes us, or flows us onwards; either way,
Expand Down Expand Up @@ -575,7 +579,7 @@ struct Heap2LocalOptimizer {
// added the parent to |seen| for both children, the reference would get
// blocked from being optimized.
if (!seen.emplace(parent).second) {
return;
return true;
}

// We can proceed, as the parent interacts with us properly, and we are
Expand Down Expand Up @@ -617,11 +621,11 @@ struct Heap2LocalOptimizer {

// We finished the loop over the flows. Do the final checks.
if (!getsAreExclusiveToSets(rewriter.sets)) {
return;
return true;
}

// We can do it, hurray!
rewriter.applyOptimization();
// Nothing escapes, hurray!
return false;
}

ParentChildInteraction getParentChildInteraction(StructNew* allocation,
Expand Down