VM: Remove PushTempInstr, simplify SSA renaming.

runtime/vm/flow_graph.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/flow_graph.h"
 
 #include "vm/bit_vector.h"
 #include "vm/cha.h"
 #include "vm/flow_graph_builder.h"
 #include "vm/flow_graph_compiler.h"
@@ skipping 1062 matching lines @@
     Instruction* current = it.Current();
 
     // Attach current environment to the instructions that need it.
     if (current->NeedsEnvironment()) {
       AttachEnvironment(current, env);
     }
 
     // 2a. Handle uses:
     // Update the expression stack renaming environment for each use by
     // removing the renamed value.
-    // For each use of a LoadLocal, StoreLocal, or Constant: Replace it with
-    // the renamed value.
+    // For each use of a LoadLocal, StoreLocal, DropTemps or Constant: Replace
+    // it with the renamed value.
     for (intptr_t i = current->InputCount() - 1; i >= 0; --i) {
       Value* v = current->InputAt(i);
       // Update expression stack.
       ASSERT(env->length() > variable_count());
 
       Definition* reaching_defn = env->RemoveLast();
       Definition* input_defn = v->definition();
       if (input_defn->IsLoadLocal() ||
           input_defn->IsStoreLocal() ||
-          input_defn->IsPushTemp() ||
           input_defn->IsDropTemps() ||
           input_defn->IsConstant()) {
-        // Remove the load/store from the graph.
-        input_defn->RemoveFromGraph();
         // Assert we are not referencing nulls in the initial environment.
         ASSERT(reaching_defn->ssa_temp_index() != -1);
         v->set_definition(reaching_defn);
         input_defn = reaching_defn;
       }

[Vyacheslav Egorov (Google), 2016/05/03 20:04:51]

I think this can be rewritten as:

if (input_defn != reaching_defn) {
  // ...
  v->set_definition(reaching_defn);
  input_defn = reaching_defn;
}

[Florian Schneider, 2016/05/09 14:47:20]

This should work - it's a little subtle. I'll add a documenting ASSERT that
input_defn is one of LoadLocal, StoreLocal, DropTemps or Constant

       input_defn->AddInputUse(v);
     }
 
     // Drop pushed arguments for calls.
     for (intptr_t j = 0; j < current->ArgumentCount(); j++) {
       env->RemoveLast();
     }
 
-    // 2b. Handle LoadLocal, StoreLocal, and Constant.
+    // 2b. Handle LoadLocal, StoreLocal, DropTemps and Constant.
     Definition* definition = current->AsDefinition();
     if (definition != NULL) {
       LoadLocalInstr* load = definition->AsLoadLocal();
       StoreLocalInstr* store = definition->AsStoreLocal();
-      PushTempInstr* push = definition->AsPushTemp();
       DropTempsInstr* drop = definition->AsDropTemps();
       ConstantInstr* constant = definition->AsConstant();
       if ((load != NULL) ||
           (store != NULL) ||
-          (push != NULL) ||
           (drop != NULL) ||
           (constant != NULL)) {
         Definition* result = NULL;
         if (store != NULL) {
           // Update renaming environment.
           intptr_t index = store->local().BitIndexIn(num_non_copied_params_);
           result = store->value()->definition();
 
           if (!FLAG_prune_dead_locals ||
               variable_liveness->IsStoreAlive(block_entry, store)) {
@@ skipping 18 matching lines @@
               variable_liveness->IsLastLoad(block_entry, load)) {
             (*env)[index] = constant_dead();
           }
 
           // Record captured parameters so that they can be skipped when
           // emitting sync code inside optimized try-blocks.
           if (load->local().is_captured_parameter()) {
             intptr_t index = load->local().BitIndexIn(num_non_copied_params_);
             captured_parameters_->Add(index);
           }
-
-        } else if (push != NULL) {
-          result = push->value()->definition();
-          env->Add(result);
-          it.RemoveCurrentFromGraph();
-          continue;
         } else if (drop != NULL) {
           // Drop temps from the environment.
           for (intptr_t j = 0; j < drop->num_temps(); j++) {
             env->RemoveLast();
           }
           if (drop->value() != NULL) {
             result = drop->value()->definition();
           }
           ASSERT((drop->value() != NULL) || !drop->HasTemp());
         } else {
           ASSERT(definition->HasTemp());
           result = GetConstant(constant->value());
         }
         // Update expression stack or remove from graph.
         if (definition->HasTemp()) {
           ASSERT(result != NULL);
           env->Add(result);
-          // We remove load/store/constant instructions when we find their
-          // use in 2a.
-        } else {
-          it.RemoveCurrentFromGraph();
         }
+        it.RemoveCurrentFromGraph();
       } else {
-        // Not a load, store, or constant.
+        // Not a load, store, drop or constant.
         if (definition->HasTemp()) {
           // Assign fresh SSA temporary and update expression stack.
           AllocateSSAIndexes(definition);
           env->Add(definition);
         }
       }
     }
 
     // 2c. Handle pushed argument.
     PushArgumentInstr* push = current->AsPushArgument();
@@ skipping 859 matching lines @@
         ReplaceCurrentInstruction(&it, current, replacement);
         changed = true;
       }
     }
   }
   return changed;
 }
 
 
 }  // namespace dart