Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1793)

Unified Diff: runtime/vm/flow_graph_optimizer.cc

Issue 10956013: Reapply "A simpler scheme for garbage collection of ureachable phi inputs." (Closed) Base URL: https://dart.googlecode.com/svn/branches/bleeding_edge/dart
Patch Set: Created 8 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: runtime/vm/flow_graph_optimizer.cc
diff --git a/runtime/vm/flow_graph_optimizer.cc b/runtime/vm/flow_graph_optimizer.cc
index 17e24d11fc445e079231e9471b43875dc1ecc1d5..0b5007264c64c8eb97c687c687180a097f98880d 100644
--- a/runtime/vm/flow_graph_optimizer.cc
+++ b/runtime/vm/flow_graph_optimizer.cc
@@ -1928,14 +1928,25 @@ void ConstantPropagator::VisitGoto(GotoInstr* instr) {
void ConstantPropagator::VisitBranch(BranchInstr* instr) {
instr->comparison()->Accept(this);
- const Object& value = instr->comparison()->constant_value();
- if (IsNonConstant(value)) {
- SetReachable(instr->true_successor());
- SetReachable(instr->false_successor());
- } else if (value.raw() == Bool::True()) {
- SetReachable(instr->true_successor());
- } else if (!IsUnknown(value)) { // Any other constant.
- SetReachable(instr->false_successor());
+
+ // The successors may be reachable, but only if this instruction is. (We
Kevin Millikin (Google) 2012/09/20 13:48:17 And look here.
+ // might be analyzing it because the constant value of one of its inputs
+ // has changed.)
+ BlockEntryInstr* entry = NULL;
Vyacheslav Egorov (Google) 2012/09/21 14:48:05 ->GetBlock() instead of custom loop
+ Instruction* prev = instr->previous();
+ while ((entry = prev->AsBlockEntry()) == NULL) {
+ prev = prev->previous();
+ }
+ if (reachable_->Contains(entry->preorder_number())) {
+ const Object& value = instr->comparison()->constant_value();
+ if (IsNonConstant(value)) {
+ SetReachable(instr->true_successor());
+ SetReachable(instr->false_successor());
+ } else if (value.raw() == Bool::True()) {
+ SetReachable(instr->true_successor());
+ } else if (!IsUnknown(value)) { // Any other constant.
+ SetReachable(instr->false_successor());
+ }
}
}
@@ -2384,6 +2395,39 @@ void ConstantPropagator::Transform() {
if (!reachable_->Contains(block->preorder_number())) {
continue;
}
+
+ JoinEntryInstr* join = block->AsJoinEntry();
+ if (join != NULL) {
+ // Remove phi inputs corresponding to unreachable predecessor blocks.
+ // Predecessors will be recomputed (in block id order) after removing
+ // unreachable code so we merely have to keep the phi inputs in order.
+ ZoneGrowableArray<PhiInstr*>* phis = join->phis();
+ if (phis != NULL) {
+ intptr_t pred_count = join->PredecessorCount();
+ intptr_t live_count = 0;
+ for (intptr_t pred_idx = 0; pred_idx < pred_count; ++pred_idx) {
+ if (reachable_->Contains(
+ join->PredecessorAt(pred_idx)->preorder_number())) {
+ if (live_count < pred_idx) {
+ for (intptr_t phi_idx = 0; phi_idx < phis->length(); ++phi_idx) {
+ PhiInstr* phi = (*phis)[phi_idx];
+ if (phi == NULL) continue;
+ phi->inputs_[live_count] = phi->inputs_[pred_idx];
+ }
+ }
+ ++live_count;
+ }
+ }
+ if (live_count < pred_count) {
+ for (intptr_t phi_idx = 0; phi_idx < phis->length(); ++phi_idx) {
+ PhiInstr* phi = (*phis)[phi_idx];
+ if (phi == NULL) continue;
+ phi->inputs_.TruncateTo(live_count);
+ }
+ }
+ }
+ }
+
for (ForwardInstructionIterator i(block); !i.Done(); i.Advance()) {
Definition* defn = i.Current()->AsDefinition();
BranchInstr* branch = i.Current()->AsBranch();
@@ -2411,13 +2455,14 @@ void ConstantPropagator::Transform() {
ASSERT(branch->comparison()->IsStrictCompare());
ASSERT(if_false->parallel_move() == NULL);
ASSERT(if_false->loop_info() == NULL);
- join = new JoinEntryInstr(if_false->try_index());
+ join =
+ new JoinEntryInstr(if_false->block_id(), if_false->try_index());
next = if_false->next();
} else if (!reachable_->Contains(if_false->preorder_number())) {
ASSERT(branch->comparison()->IsStrictCompare());
ASSERT(if_true->parallel_move() == NULL);
ASSERT(if_true->loop_info() == NULL);
- join = new JoinEntryInstr(if_true->try_index());
+ join = new JoinEntryInstr(if_true->block_id(), if_true->try_index());
next = if_true->next();
}
@@ -2444,19 +2489,6 @@ void ConstantPropagator::Transform() {
graph_->DiscoverBlocks();
GrowableArray<BitVector*> dominance_frontier;
graph_->ComputeDominators(&dominance_frontier);
-
- // Garbage collect phi inputs corresponding to unreachable predecessors.
- // This is required because we assume that predecessor and phi indexes
- // align. Note that this does not necessarily eliminate all useless phis
- // (e.g., it does not eliminate phis that were originally inserted solely
- // due to an assignment on the now-unreachable path).
- for (BlockIterator it = graph_->reverse_postorder_iterator();
- !it.Done();
- it.Advance()) {
- JoinEntryInstr* join = it.Current()->AsJoinEntry();
- if (join != NULL) join->EliminateUnreachablePhiInputs();
- }
-
graph_->ComputeUseLists();
}

Powered by Google App Engine
This is Rietveld 408576698