Factored out common Instruction code in the CFG builder that depends only...

src/cfg.cc

 // Copyright 2009 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 182 matching lines @@
     }
     PrintF("--- Code ---\n");
     code->Disassemble(*fun->name()->ToCString());
   }
 #endif
 
   return code;
 }
 
 
-void MoveInstr::FastAllocate(TempLocation* temp) {
-  ASSERT(temp->where() == TempLocation::NOT_ALLOCATED);
-  if (temp == value()) {
-    temp->set_where(TempLocation::ACCUMULATOR);
-  } else {
-    temp->set_where(TempLocation::STACK);
-  }
+void ZeroOperandInstruction::FastAllocate(TempLocation* temp) {
+  temp->set_where(TempLocation::STACK);
 }
 
 
-void PropLoadInstr::FastAllocate(TempLocation* temp) {
-  ASSERT(temp->where() == TempLocation::NOT_ALLOCATED);
-  if (temp == object() || temp == key()) {
-    temp->set_where(TempLocation::ACCUMULATOR);
-  } else {
-    temp->set_where(TempLocation::STACK);
-  }
+void OneOperandInstruction::FastAllocate(TempLocation* temp) {
+  temp->set_where((temp == value_)
+                  ? TempLocation::ACCUMULATOR
+                  : TempLocation::STACK);
 }
 
 
-void BinaryOpInstr::FastAllocate(TempLocation* temp) {
-  ASSERT(temp->where() == TempLocation::NOT_ALLOCATED);
-  if (temp == left() || temp == right()) {
-    temp->set_where(TempLocation::ACCUMULATOR);
-  } else {
-    temp->set_where(TempLocation::STACK);
-  }
-}
-
-
-void ReturnInstr::FastAllocate(TempLocation* temp) {
-  ASSERT(temp->where() == TempLocation::NOT_ALLOCATED);
-  if (temp == value()) {
-    temp->set_where(TempLocation::ACCUMULATOR);
-  } else {
-    temp->set_where(TempLocation::STACK);
-  }
+void TwoOperandInstruction::FastAllocate(TempLocation* temp) {
+  temp->set_where((temp == value0_ || temp == value1_)
+                  ? TempLocation::ACCUMULATOR
+                  : TempLocation::STACK);
 }
 
 
 void PositionInstr::Compile(MacroAssembler* masm) {
   if (FLAG_debug_info && pos_ != RelocInfo::kNoPosition) {
     masm->RecordStatementPosition(pos_);
     masm->RecordPosition(pos_);
   }
 }
 
@@ skipping 382 matching lines @@
 #ifdef DEBUG
 // CFG printing support (via depth-first, preorder block traversal).
 
 void Cfg::Print() {
   entry_->Print();
   entry_->Unmark();
 }
 
 
 void Constant::Print() {
-  PrintF("Constant(");
+  PrintF("Constant ");
   handle_->Print();
-  PrintF(")");
 }
 
 
 void Nowhere::Print() {
   PrintF("Nowhere");
 }
 
 
 void SlotLocation::Print() {
-  PrintF("Slot(");
+  PrintF("Slot ");
   switch (type_) {
     case Slot::PARAMETER:
-      PrintF("PARAMETER, %d)", index_);
+      PrintF("(PARAMETER, %d)", index_);
       break;
     case Slot::LOCAL:
-      PrintF("LOCAL, %d)", index_);
+      PrintF("(LOCAL, %d)", index_);
       break;
     default:
       UNREACHABLE();
   }
 }
 
 
 void TempLocation::Print() {
-  PrintF("Temp(%d)", number());
+  PrintF("Temp %d", number());
+}
+
+
+void OneOperandInstruction::Print() {
+  PrintF("(");
+  location()->Print();
+  PrintF(", ");
+  value_->Print();
+  PrintF(")");
+}
+
+
+void TwoOperandInstruction::Print() {
+  PrintF("(");
+  location()->Print();
+  PrintF(", ");
+  value0_->Print();
+  PrintF(", ");
+  value1_->Print();
+  PrintF(")");
 }
 
 
 void MoveInstr::Print() {
-  PrintF("Move(");
-  location()->Print();
-  PrintF(", ");
-  value_->Print();
-  PrintF(")\n");
+  PrintF("Move              ");
+  OneOperandInstruction::Print();
+  PrintF("\n");
 }
 
 
 void PropLoadInstr::Print() {
-  PrintF("PropLoad(");
-  location()->Print();
-  PrintF(", ");
-  object()->Print();
-  PrintF(", ");
-  key()->Print();
-  PrintF(")\n");
+  PrintF("PropLoad          ");
+  TwoOperandInstruction::Print();
+  PrintF("\n");
 }
 
 
 void BinaryOpInstr::Print() {
-  PrintF("BinaryOp(");
-  location()->Print();
-  PrintF(", %s, ", Token::Name(op()));
-  left()->Print();
-  PrintF(", ");
-  right()->Print();
-  PrintF(")\n");
+  switch (op()) {
+    case Token::OR:
+      // Two character operand.
+      PrintF("BinaryOp[OR]      ");
+      break;
+    case Token::AND:
+    case Token::SHL:
+    case Token::SAR:
+    case Token::SHR:
+    case Token::ADD:
+    case Token::SUB:
+    case Token::MUL:
+    case Token::DIV:
+    case Token::MOD:
+      // Three character operands.
+      PrintF("BinaryOp[%s]     ", Token::Name(op()));
+      break;
+    case Token::COMMA:
+      // Five character operand.
+      PrintF("BinaryOp[COMMA]   ");
+      break;
+    case Token::BIT_OR:
+      // Six character operand.
+      PrintF("BinaryOp[BIT_OR]  ");
+      break;
+    case Token::BIT_XOR:
+    case Token::BIT_AND:
+      // Seven character operands.
+      PrintF("BinaryOp[%s] ", Token::Name(op()));

[William Hesse, 2009/08/10 12:47:08]

If we print this on two lines, or don't print "BinaryOp", the instruction name
field can get much shorter.  I think that would be good.

[Kevin Millikin (Chromium), 2009/08/10 12:59:11]

Printing on two lines seems annoying.  I'd much rather keep one instruction per
line.

I prefer painful explicitness (and accuracy) about the actual object structure
involved, since it's intended for debug printing.

+      break;
+    default:
+      UNREACHABLE();
+  }
+  TwoOperandInstruction::Print();
+  PrintF("\n");
 }
 
 
 void ReturnInstr::Print() {
-  PrintF("Return(");
-  value_->Print();
-  PrintF(")\n");
+  PrintF("Return            ");
+  OneOperandInstruction::Print();
+  PrintF("\n");
 }
 
 
 void InstructionBlock::Print() {
   if (!is_marked_) {
     is_marked_ = true;
     PrintF("L%d:\n", number());

[William Hesse, 2009/08/10 12:47:08]

number is a pretty non-descriptive name for the node number.

[Kevin Millikin (Chromium), 2009/08/10 12:59:11]

CfgNode::number() doesn't seem non-descriptive for the node number.  I'd change
it to something else if there ever became a danger of confusion, but otherwise I
prefer the simplicity.

     for (int i = 0, len = instructions_.length(); i < len; i++) {
       instructions_[i]->Print();
     }
-    PrintF("Goto L%d\n\n", successor_->number());
+    PrintF("Goto              L%d\n\n", successor_->number());
     successor_->Print();
   }
 }
 
 
 void EntryNode::Print() {
   if (!is_marked_) {
     is_marked_ = true;
     successor_->Print();
   }
 }
 
 
 void ExitNode::Print() {
   if (!is_marked_) {
     is_marked_ = true;
     PrintF("L%d:\nExit\n\n", number());
   }
 }
 
 #endif  // DEBUG
 
 } }  // namespace v8::internal