Nested deoptimization environments.

runtime/vm/flow_graph_allocator.cc

Index: runtime/vm/flow_graph_allocator.cc
diff --git a/runtime/vm/flow_graph_allocator.cc b/runtime/vm/flow_graph_allocator.cc
index a3664ede81aa48f9131678830e93a257942aff32..0cf0058cbb2d63230dd9963bdf435516bbddf6ba 100644
--- a/runtime/vm/flow_graph_allocator.cc
+++ b/runtime/vm/flow_graph_allocator.cc
@@ -95,19 +95,18 @@ void FlowGraphAllocator::EliminateEnvironmentUses() {
       Instruction* current = it.Current();
       if (current->CanDeoptimize()) {
         ASSERT(current->env() != NULL);
-        GrowableArray<Value*>* values = current->env()->values_ptr();
-        for (intptr_t i = 0; i < values->length(); i++) {
-          Value* use = (*values)[i];
+        for (EnvironmentIterator it(current->env()); !it.Done(); it.Advance()) {
+          Value* use = it.CurrentValue();
           Definition* def = use->definition();
           PushArgumentInstr* push_argument = def->AsPushArgument();
           if ((push_argument != NULL) && push_argument->WasEliminated()) {
-            (*values)[i] = push_argument->value()->Copy();
+            it.ReplaceValue(push_argument->value()->Copy());
             continue;
           }
 
           PhiInstr* phi = def->AsPhi();
           if ((phi != NULL) && !phi->is_alive()) {
-            (*values)[i] = new Value(constant_null);
+            it.ReplaceValue(new Value(constant_null));
             continue;
           }
         }
@@ -147,9 +146,8 @@ void FlowGraphAllocator::ComputeInitialSets() {
 
       // Add uses from the deoptimization environment.
       if (current->env() != NULL) {
-        const GrowableArray<Value*>& values = current->env()->values();
-        for (intptr_t j = 0; j < values.length(); j++) {
-          Value* value = values[j];
+        for (intptr_t i = 0; i < current->env()->Length(); ++i) {

[Kevin Millikin (Google), 2012/09/07 12:51:52]

This seems wrong.  Why are values in outer environments not used here?

[zerny-google, 2012/09/07 13:51:37]

I think this one is safe, but I have tried only to iterate the entire env. in
safe locations. I'll update to iterators/recursive procedures in the
inline-deopt CL.

+          Value* value = current->env()->ValueAt(i);
           if (!value->definition()->IsPushArgument()) {
             live_in->Add(value->definition()->ssa_temp_index());
           }
@@ -185,8 +183,8 @@ void FlowGraphAllocator::ComputeInitialSets() {
 
   // Process incoming parameters.
   GraphEntryInstr* graph_entry = postorder_.Last()->AsGraphEntry();
-  for (intptr_t i = 0; i < graph_entry->start_env()->values().length(); i++) {
-    Value* val = graph_entry->start_env()->values()[i];
+  for (intptr_t i = 0; i < graph_entry->start_env()->Length(); i++) {
+    Value* val = graph_entry->start_env()->ValueAt(i);
     intptr_t vreg = val->definition()->ssa_temp_index();
     kill_[graph_entry->postorder_number()]->Add(vreg);
     live_in_[graph_entry->postorder_number()]->Remove(vreg);
@@ -519,8 +517,8 @@ void FlowGraphAllocator::BuildLiveRanges() {
   // Process incoming parameters.  Do this after all other instructions so
   // that safepoints for all calls have already been found.
   GraphEntryInstr* graph_entry = postorder_.Last()->AsGraphEntry();
-  for (intptr_t i = 0; i < graph_entry->start_env()->values().length(); i++) {
-    Value* val = graph_entry->start_env()->values()[i];
+  for (intptr_t i = 0; i < graph_entry->start_env()->Length(); i++) {
+    Value* val = graph_entry->start_env()->ValueAt(i);
     ParameterInstr* param = val->definition()->AsParameter();
     if (param == NULL) continue;
 
@@ -748,17 +746,16 @@ void FlowGraphAllocator::ProcessEnvironmentUses(BlockEntryInstr* block,
   //      value    -----*
   //
 
-  const GrowableArray<Value*>& values = env->values();
-  if (values.length() == 0) return;
+  if (env->Length() == 0) return;

[Kevin Millikin (Google), 2012/09/07 12:51:52]

This also seems wrong.  What if there are values in outer environments?

[zerny-google, 2012/09/07 13:51:37]

This needs to be defined recursively on the nested envs, since locations are
attached at each individual env.

 
   const intptr_t block_start_pos = block->start_pos();
   const intptr_t use_pos = current->lifetime_position() + 1;
 
   Location* locations =
-      Isolate::Current()->current_zone()->Alloc<Location>(values.length());
+      Isolate::Current()->current_zone()->Alloc<Location>(env->Length());
 
-  for (intptr_t i = 0; i < values.length(); ++i) {
-    Value* value = values[i];
+  for (intptr_t i = 0; i < env->Length(); ++i) {
+    Value* value = env->ValueAt(i);
     locations[i] = Location::Any();
     Definition* def = value->definition();