[turbofan] improved move optimizer readability: MoveKey

src/compiler/move-optimizer.cc

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/compiler/move-optimizer.h"
 
 namespace v8 {
 namespace internal {
 namespace compiler {
 
 namespace {
 
-typedef std::pair<InstructionOperand, InstructionOperand> MoveKey;
+struct MoveKey {
+  InstructionOperand source_;

[Jarin, 2016/01/27 05:11:50]

You should not have underscores at the end for public fields in structs.

[Mircea Trofin, 2016/01/27 06:37:43]

Oh! Happy to remove them! 

Some parts of the code have them, which is what got me down this path - see
PrintableInstructionOperand for instance. I suppose this is something we'd want
to clean up?

+  InstructionOperand destination_;
+};
 
 struct MoveKeyCompare {
   bool operator()(const MoveKey& a, const MoveKey& b) const {
-    if (a.first.EqualsCanonicalized(b.first)) {
-      return a.second.CompareCanonicalized(b.second);
+    if (a.source_.EqualsCanonicalized(b.source_)) {
+      return a.destination_.CompareCanonicalized(b.destination_);
     }
-    return a.first.CompareCanonicalized(b.first);
+    return a.source_.CompareCanonicalized(b.source_);
   }
 };
 
 struct OperandCompare {
   bool operator()(const InstructionOperand& a,
                   const InstructionOperand& b) const {
     return a.CompareCanonicalized(b);
   }
 };
 
@@ skipping 235 matching lines @@
   DCHECK_NOT_NULL(instr);
 
   if (correct_counts != move_map.size()) {
     // Moves that are unique to each predecessor won't be pushed to the common
     // successor.
     OperandSet conflicting_srcs(local_zone());
     for (auto iter = move_map.begin(), end = move_map.end(); iter != end;) {
       auto current = iter;
       ++iter;
       if (current->second != block->PredecessorCount()) {
-        InstructionOperand dest = current->first.second;
+        InstructionOperand dest = current->first.destination_;
         // Not all the moves in all the gaps are the same. Maybe some are. If
         // there are such moves, we could move them, but the destination of the
         // moves staying behind can't appear as a source of a common move,
         // because the move staying behind will clobber this destination.
         conflicting_srcs.insert(dest);
         move_map.erase(current);
       }
     }
 
     bool changed = false;
     do {
       // If a common move can't be pushed to the common successor, then its
       // destination also can't appear as source to any move being pushed.
       changed = false;
       for (auto iter = move_map.begin(), end = move_map.end(); iter != end;) {
         auto current = iter;
         ++iter;
         DCHECK_EQ(block->PredecessorCount(), current->second);
-        if (conflicting_srcs.find(current->first.first) !=
+        if (conflicting_srcs.find(current->first.source_) !=
             conflicting_srcs.end()) {
-          conflicting_srcs.insert(current->first.second);
+          conflicting_srcs.insert(current->first.destination_);
           move_map.erase(current);
           changed = true;
         }
       }
     } while (changed);
   }
 
   if (move_map.empty()) return;
 
   DCHECK_NOT_NULL(instr);
@@ skipping 83 matching lines @@
         static_cast<Instruction::GapPosition>(1), code_zone());
     slot_1->AddMove(group_begin->destination(), load->destination());
     load->Eliminate();
   }
   loads.clear();
 }
 
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8