Optimize functions containing try-catch.

runtime/vm/flow_graph_optimizer.cc

Index: runtime/vm/flow_graph_optimizer.cc
===================================================================
--- runtime/vm/flow_graph_optimizer.cc	(revision 22436)
+++ runtime/vm/flow_graph_optimizer.cc	(working copy)
@@ -15,6 +15,7 @@
 #include "vm/parser.h"
 #include "vm/resolver.h"
 #include "vm/scopes.h"
+#include "vm/stack_frame.h"
 #include "vm/symbols.h"
 
 namespace dart {
@@ -457,6 +458,14 @@
         InsertConversionsFor(phi);
       }
     }
+    CatchBlockEntryInstr* catch_entry = entry->AsCatchBlockEntry();
+    if (catch_entry != NULL) {
+      for (intptr_t i = 0;
+           i < catch_entry->initial_definitions()->length();
+           i++) {
+        InsertConversionsFor((*catch_entry->initial_definitions())[i]);
+      }
+    }
     for (ForwardInstructionIterator it(entry); !it.Done(); it.Advance()) {
       Definition* def = it.Current()->AsDefinition();
       if (def != NULL) {
@@ -3136,6 +3145,68 @@
 }
 
 
+void FlowGraphOptimizer::AnalyzeTryCatch() {

[Kevin Millikin (Google), 2013/05/08 11:42:00]

Doesn't need to be in the FlowGraphOptimizer, which is a visitor that does
ApplyICData.

I think it's confusing to put too many unrelated passes into that same visitor
class.

[Florian Schneider, 2013/05/08 17:10:55]

Done.

+  // For every catch-block: Iterate over all call instructions inside the
+  // corresponding try-block and figure out for each environment value if it
+  // is the same constant at all calls. If yes, replace the initial definition
+  // at the catch-entry with this constant.
+  const GrowableArray<CatchBlockEntryInstr*>& catch_entries =
+      flow_graph_->graph_entry()->catch_entries();
+  intptr_t nncp = flow_graph_->num_non_copied_params();

[Kevin Millikin (Google), 2013/05/08 11:42:00]

intptr_t base = kFirstLocalSlotIndex + flow_graph_->num_non_copied_params();

[Florian Schneider, 2013/05/08 17:10:55]

Done.

+  for (intptr_t catch_idx = 0;
+       catch_idx < catch_entries.length();
+       ++catch_idx) {
+    CatchBlockEntryInstr* cb = catch_entries[catch_idx];
+    CatchEntryInstr* catch_entry = cb->next()->AsCatchEntry();
+    intptr_t ex_idx =

[Kevin Millikin (Google), 2013/05/08 11:42:00]

intptr_t ex_idx = base - catch_entry->exception_var().index();
intptr_t st_idx = base - catch_entry->stacktrace_var().index();

[Florian Schneider, 2013/05/08 17:10:55]

Done.

+        kFirstLocalSlotIndex - catch_entry->exception_var().index() + nncp;
+    intptr_t st_idx =
+        kFirstLocalSlotIndex - catch_entry->stacktrace_var().index() + nncp;
+    GrowableArray<Definition*>* idefs = cb->initial_definitions();
+    GrowableArray<Definition*> cdefs(idefs->length());
+    cdefs.AddArray(*idefs);
+
+    for (BlockIterator block_it = flow_graph_->reverse_postorder_iterator();

[Kevin Millikin (Google), 2013/05/08 11:42:00]

This is doing constant analysis with the lattice:

known to be non-constant (rep: NULL)
  |
known constant (rep: ConstantInstr)
  |
unknown constant (rep: Parameter Instr)

[Florian Schneider, 2013/05/08 17:10:55]

Done.

+         !block_it.Done();
+         block_it.Advance()) {
+      BlockEntryInstr* block = block_it.Current();
+      if (block->try_index() == cb->catch_try_index()) {
+        for (ForwardInstructionIterator instr_it(block);
+             !instr_it.Done();
+             instr_it.Advance()) {
+          Instruction* current = instr_it.Current();
+          if (current->MayThrow()) {
+            Environment* env = current->env();
+            for (intptr_t env_idx = 0; env_idx < cdefs.length(); ++env_idx) {
+              if (cdefs[env_idx] != NULL &&
+                  cdefs[env_idx]->IsParameter() &&
+                  cdefs[env_idx]->AsParameter()->index() != ex_idx &&

[Kevin Millikin (Google), 2013/05/08 11:42:00]

The intent is that ex_idx and st_idx are not constant.  I think it's simpler to
set them to NULL before performing the analysis and have no special treatment in
the loop.

[Florian Schneider, 2013/05/08 17:10:55]

Done.

+                  cdefs[env_idx]->AsParameter()->index() != st_idx &&
+                  env->ValueAt(env_idx)->definition()->IsConstant()) {

[Kevin Millikin (Google), 2013/05/08 11:42:00]

I guess you can write this as env->ValueAt(env_idx)->BindsToConstant();

[Florian Schneider, 2013/05/08 17:10:55]

Done.

+                cdefs[env_idx] = env->ValueAt(env_idx)->definition();
+              }
+              if (cdefs[env_idx] != env->ValueAt(env_idx)->definition()) {
+                cdefs[env_idx] = NULL;
+              }
+            }
+          }
+        }
+      }
+    }
+    for (intptr_t j = 0; j < idefs->length(); ++j) {
+      if (cdefs[j] != NULL && cdefs[j]->IsConstant()) {
+        Definition* old = (*idefs)[j];
+        ConstantInstr* orig = cdefs[j]->AsConstant();
+        ConstantInstr* copy = new ConstantInstr(orig->value());

[Kevin Millikin (Google), 2013/05/08 11:42:00]

Note that all constants are pooled now.  I think you can simply replaces uses
with the one from the pool and remove this definition from the catch block's
initial definitions.  Will that work?

[Florian Schneider, 2013/05/08 17:10:55]

Yes. It should and make the code much simpler here. Added a TODO for now.

+        copy->set_ssa_temp_index(flow_graph_->alloc_ssa_temp_index());
+        old->ReplaceUsesWith(copy);
+        (*idefs)[j] = copy;
+      }
+    }
+  }
+}
+
+
 static BlockEntryInstr* FindPreHeader(BlockEntryInstr* header) {
   for (intptr_t j = 0; j < header->PredecessorCount(); ++j) {
     BlockEntryInstr* candidate = header->PredecessorAt(j);
@@ -4398,7 +4469,9 @@
   }
   ASSERT(ForwardInstructionIterator(block).Done());
 
-  SetReachable(block->normal_entry());
+  for (intptr_t i = 0; i < block->SuccessorCount(); ++i) {
+    SetReachable(block->SuccessorAt(i));

[Kevin Millikin (Google), 2013/05/08 11:42:00]

Hmm, this is a poor approximation.  Can you add a comment that a better one is
that the catch is reachable if the try entry is (actually, if a call inside the
try is).

[Florian Schneider, 2013/05/08 17:10:55]

Done.

+  }
 }
 
 
@@ -4418,6 +4491,10 @@
 
 
 void ConstantPropagator::VisitCatchBlockEntry(CatchBlockEntryInstr* block) {
+  const GrowableArray<Definition*>& defs = *block->initial_definitions();
+  for (intptr_t i = 0; i < defs.length(); ++i) {
+    defs[i]->Accept(this);
+  }
   for (ForwardInstructionIterator it(block); !it.Done(); it.Advance()) {
     it.Current()->Accept(this);
   }
@@ -5224,12 +5301,16 @@
 void ConstantPropagator::VisitBranches() {
   GraphEntryInstr* entry = graph_->graph_entry();
   reachable_->Add(entry->preorder_number());
-  // TODO(fschneider): Handle CatchEntry.
-  reachable_->Add(entry->normal_entry()->preorder_number());
-  block_worklist_.Add(entry->normal_entry());
+  block_worklist_.Add(entry);
 
   while (!block_worklist_.is_empty()) {
     BlockEntryInstr* block = block_worklist_.RemoveLast();
+    if (block->IsGraphEntry()) {
+      for (intptr_t i = 0; i < block->SuccessorCount(); ++i) {
+        SetReachable(block->SuccessorAt(i));
+      }
+      continue;
+    }
     Instruction* last = block->last_instruction();
     if (last->IsGoto()) {
       SetReachable(last->AsGoto()->successor());