[interpreter] Faster and fewer flushes in register optimizer.

src/interpreter/bytecode-register-optimizer.cc

Index: src/interpreter/bytecode-register-optimizer.cc
diff --git a/src/interpreter/bytecode-register-optimizer.cc b/src/interpreter/bytecode-register-optimizer.cc
index c7578b4b101f7d76a60144300d378683e495e4c0..a100b5872a21871e9243b3fd86d5a6778d960024 100644
--- a/src/interpreter/bytecode-register-optimizer.cc
+++ b/src/interpreter/bytecode-register-optimizer.cc
@@ -48,6 +48,9 @@ class BytecodeRegisterOptimizer::RegisterInfo final : public ZoneObject {
   // exists.
   RegisterInfo* GetEquivalentToMaterialize();
 
+  // Get an equivalent register. Returns this if none exists.
+  RegisterInfo* GetEquivalent();
+
   Register register_value() const { return register_; }
   bool materialized() const { return materialized_; }
   void set_materialized(bool materialized) { materialized_ = materialized; }
@@ -161,6 +164,11 @@ BytecodeRegisterOptimizer::RegisterInfo::GetEquivalentToMaterialize() {
   return best_info;
 }
 
+BytecodeRegisterOptimizer::RegisterInfo*
+BytecodeRegisterOptimizer::RegisterInfo::GetEquivalent() {
+  return next_;
+}
+
 BytecodeRegisterOptimizer::BytecodeRegisterOptimizer(
     Zone* zone, TemporaryRegisterAllocator* register_allocator,
     int parameter_count, BytecodePipelineStage* next_stage)
@@ -169,7 +177,7 @@ BytecodeRegisterOptimizer::BytecodeRegisterOptimizer(
       register_info_table_(zone),
       equivalence_id_(0),
       next_stage_(next_stage),
-      flushed_(false),
+      flush_required_(false),
       zone_(zone) {
   register_allocator->set_observer(this);
 
@@ -199,31 +207,30 @@ BytecodeRegisterOptimizer::BytecodeRegisterOptimizer(
 }
 
 void BytecodeRegisterOptimizer::FlushState() {
-  if (flushed_) {
+  if (!flush_required_) {
     return;
   }
 
-  // Materialize all live registers.
+  // Materialize all live registers and break equivalences.
   size_t count = register_info_table_.size();
   for (size_t i = 0; i < count; ++i) {
     RegisterInfo* reg_info = register_info_table_[i];
-    if (!reg_info->IsOnlyMemberOfEquivalenceSet() &&
-        !reg_info->materialized()) {
-      DCHECK(RegisterIsTemporary(reg_info->register_value()) ||
-             reg_info->register_value() == accumulator_);
-      Materialize(reg_info);
+    if (reg_info->IsOnlyMemberOfEquivalenceSet() || !reg_info->materialized()) {
+      continue;

[rmcilroy, 2016/06/02 12:51:33]

nit - could you add a comment here that you are skipping non-materialized reg
info, and they will be materialized when a materialized equivalent is reached.

[oth, 2016/06/02 14:14:24]

Done.

     }
-  }
 
-  // Break all existing equivalences.
-  for (size_t i = 0; i < count; ++i) {
-    RegisterInfo* reg_info = register_info_table_[i];
-    if (!reg_info->IsOnlyMemberOfEquivalenceSet()) {
-      reg_info->MoveToNewEquivalenceSet(NextEquivalenceId(), true);
+    // Walk equivalents of this materialized register, materializing
+    // each equivalent if necessary and moving to own equivalent set.
+    RegisterInfo* equivalent;
+    while ((equivalent = reg_info->GetEquivalent()) != reg_info) {
+      if (!equivalent->materialized()) {
+        OutputRegisterTransfer(reg_info, equivalent);
+      }
+      equivalent->MoveToNewEquivalenceSet(NextEquivalenceId(), true);
     }
   }
 
-  flushed_ = true;
+  flush_required_ = false;
 }
 
 // override
@@ -240,8 +247,6 @@ size_t BytecodeRegisterOptimizer::FlushForOffset() {
 
 // override
 void BytecodeRegisterOptimizer::Write(BytecodeNode* node) {
-  flushed_ = false;
-
   //
   // Transfers with observable registers as the destination will be
   // immediately materialized so the source position information will
@@ -370,6 +375,7 @@ void BytecodeRegisterOptimizer::RegisterTransfer(
   // Add |output_info| to new equivalence set.
   if (!output_info->IsInSameEquivalenceSet(input_info)) {
     output_info->AddToEquivalenceSetOf(input_info);
+    flush_required_ = true;

[rmcilroy, 2016/06/02 12:51:34]

This scares me a bit since if we add another call to ddToEquivalenceSetOf
somewhere else and forget the flush_required_ things will break. Maybe add a
helper function which does both operations and call this from here?

[oth, 2016/06/02 14:14:24]

Done.

If flushes aren't correctly emitted many tests fail. 

We've consciously made a common path for the register transfer bytecodes so I'm
unclear why we'd ever have another path doing this operation.

[rmcilroy, 2016/06/02 15:07:13]

Acknowledged, I'm fine with this if you are.

   }
 
   bool output_is_observable =