Clean up the Result class. Reduce the size of Result from four words

to one by keeping a stack of active code generators and by using
indirection to handles.

Mainly a cleanup.  No visible performance impact.
Review URL: http://codereview.chromium.org/113455

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@1965 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/codegen.cc

@@ -40,6 +40,10 @@
 
 namespace v8 { namespace internal {
 
+
+CodeGenerator* CodeGeneratorScope::top_ = NULL;
+
+
 DeferredCode::DeferredCode(CodeGenerator* generator)
   : generator_(generator),
     masm_(generator->masm()),
@@ -154,6 +158,7 @@ Handle<Code> CodeGenerator::MakeCode(FunctionLiteral* flit,
   // Generate code.
   const int initial_buffer_size = 4 * KB;
   CodeGenerator cgen(initial_buffer_size, script, is_eval);
+  CodeGeneratorScope scope(&cgen);
   cgen.GenCode(flit);
   if (cgen.HasStackOverflow()) {
     ASSERT(!Top::has_pending_exception());

src/codegen.h

@@ -86,9 +86,35 @@ enum OverwriteMode { NO_OVERWRITE, OVERWRITE_LEFT, OVERWRITE_RIGHT };
 #include "arm/codegen-arm.h"
 #endif
 
+#include "register-allocator.h"
+
 namespace v8 { namespace internal {
 
 
+// Code generation can be nested.  Code generation scopes form a stack
+// of active code generators.
+class CodeGeneratorScope BASE_EMBEDDED {
+ public:
+  explicit CodeGeneratorScope(CodeGenerator* cgen) {
+    previous_ = top_;
+    top_ = cgen;
+  }
+
+  ~CodeGeneratorScope() {
+    top_ = previous_;
+  }
+
+  static CodeGenerator* Current() {
+    ASSERT(top_ != NULL);
+    return top_;
+  }
+
+ private:
+  static CodeGenerator* top_;
+  CodeGenerator* previous_;
+};
+
+
 // Use lazy compilation; defaults to true.
 // NOTE: Do not remove non-lazy compilation until we can properly
 //       install extensions with lazy compilation enabled. At the

src/compiler.h

@@ -79,7 +79,10 @@ class CompilationZoneScope : public ZoneScope {
  public:
   explicit CompilationZoneScope(ZoneScopeMode mode) : ZoneScope(mode) { }
   virtual ~CompilationZoneScope() {
-    if (ShouldDeleteOnExit()) FrameElement::ClearConstantList();
+    if (ShouldDeleteOnExit()) {
+      FrameElement::ClearConstantList();
+      Result::ClearConstantList();
+    }
   }
 };
 

src/ia32/register-allocator-ia32.cc

@@ -38,12 +38,13 @@ namespace v8 { namespace internal {
 void Result::ToRegister() {
   ASSERT(is_valid());
   if (is_constant()) {
-    Result fresh = cgen_->allocator()->Allocate();
+    Result fresh = CodeGeneratorScope::Current()->allocator()->Allocate();
     ASSERT(fresh.is_valid());
-    if (cgen_->IsUnsafeSmi(handle())) {
-      cgen_->LoadUnsafeSmi(fresh.reg(), handle());
+    if (CodeGeneratorScope::Current()->IsUnsafeSmi(handle())) {
+      CodeGeneratorScope::Current()->LoadUnsafeSmi(fresh.reg(), handle());
     } else {
-      cgen_->masm()->Set(fresh.reg(), Immediate(handle()));
+      CodeGeneratorScope::Current()->masm()->Set(fresh.reg(),
+                                                 Immediate(handle()));
     }
     // This result becomes a copy of the fresh one.
     *this = fresh;
@@ -55,23 +56,24 @@ void Result::ToRegister() {
 void Result::ToRegister(Register target) {
   ASSERT(is_valid());
   if (!is_register() || !reg().is(target)) {
-    Result fresh = cgen_->allocator()->Allocate(target);
+    Result fresh = CodeGeneratorScope::Current()->allocator()->Allocate(target);
     ASSERT(fresh.is_valid());
     if (is_register()) {
-      cgen_->masm()->mov(fresh.reg(), reg());
+      CodeGeneratorScope::Current()->masm()->mov(fresh.reg(), reg());
     } else {
       ASSERT(is_constant());
-      if (cgen_->IsUnsafeSmi(handle())) {
-        cgen_->LoadUnsafeSmi(fresh.reg(), handle());
+      if (CodeGeneratorScope::Current()->IsUnsafeSmi(handle())) {
+        CodeGeneratorScope::Current()->LoadUnsafeSmi(fresh.reg(), handle());
       } else {
-        cgen_->masm()->Set(fresh.reg(), Immediate(handle()));
+        CodeGeneratorScope::Current()->masm()->Set(fresh.reg(),
+                                                   Immediate(handle()));
       }
     }
     *this = fresh;
   } else if (is_register() && reg().is(target)) {
-    ASSERT(cgen_->has_valid_frame());
-    cgen_->frame()->Spill(target);
-    ASSERT(cgen_->allocator()->count(target) == 1);
+    ASSERT(CodeGeneratorScope::Current()->has_valid_frame());
+    CodeGeneratorScope::Current()->frame()->Spill(target);
+    ASSERT(CodeGeneratorScope::Current()->allocator()->count(target) == 1);
   }
   ASSERT(is_register());
   ASSERT(reg().is(target));
@@ -127,7 +129,7 @@ Result RegisterAllocator::AllocateByteRegisterWithoutSpilling() {
   // register if valid and return an invalid result.
   if (result.is_valid() && !result.reg().is_byte_register()) {
     result.Unuse();
-    return Result(cgen_);
+    return Result();
   }
   return result;
 }

src/ia32/virtual-frame-ia32.cc

@@ -1075,12 +1075,12 @@ Result VirtualFrame::Pop() {
     new_element.set_static_type(element.static_type());
     elements_[index] = new_element;
     __ mov(temp.reg(), Operand(ebp, fp_relative(index)));
-    return Result(temp.reg(), cgen_, element.static_type());
+    return Result(temp.reg(), element.static_type());
   } else if (element.is_register()) {
-    return Result(element.reg(), cgen_, element.static_type());
+    return Result(element.reg(), element.static_type());
   } else {
     ASSERT(element.is_constant());
-    return Result(element.handle(), cgen_);
+    return Result(element.handle());
   }
 }
 

src/ia32/virtual-frame-ia32.h

@@ -187,7 +187,7 @@ class VirtualFrame : public ZoneObject {
 
   // Set a frame element to a constant.  The index is frame-top relative.
   void SetElementAt(int index, Handle<Object> value) {
-    Result temp(value, cgen_);
+    Result temp(value);
     SetElementAt(index, &temp);
   }
 

src/register-allocator-inl.h

@@ -35,13 +35,25 @@
 namespace v8 { namespace internal {
 
 Result::~Result() {
-  if (is_register()) cgen_->allocator()->Unuse(reg());
+  if (is_register()) {
+    CodeGeneratorScope::Current()->allocator()->Unuse(reg());
+  }
 }
 
 
 void Result::Unuse() {
-  if (is_register()) cgen_->allocator()->Unuse(reg());
-  type_ = INVALID;
+  if (is_register()) {
+    CodeGeneratorScope::Current()->allocator()->Unuse(reg());
+  }
+  invalidate();
+}
+
+
+void Result::CopyTo(Result* destination) const {
+  destination->value_ = value_;
+  if (is_register()) {
+    CodeGeneratorScope::Current()->allocator()->Use(reg());
+  }
 }
 
 

src/register-allocator.cc

@@ -35,39 +35,22 @@ namespace v8 { namespace internal {
 // -------------------------------------------------------------------------
 // Result implementation.
 
-Result::Result(Register reg, CodeGenerator* cgen)
-    : static_type_(),
-      type_(REGISTER),
-      cgen_(cgen) {
-  data_.reg_ = reg;
-  ASSERT(reg.is_valid());
-  cgen_->allocator()->Use(reg);
-}
-
 
-Result::Result(Register reg, CodeGenerator* cgen, StaticType static_type)
-    : static_type_(static_type),
-      type_(REGISTER),
-      cgen_(cgen) {
-  data_.reg_ = reg;
+Result::Result(Register reg) {
   ASSERT(reg.is_valid());
-  cgen_->allocator()->Use(reg);
+  CodeGeneratorScope::Current()->allocator()->Use(reg);
+  value_ = StaticTypeField::encode(StaticType::UNKNOWN_TYPE)
+      | TypeField::encode(REGISTER)
+      | DataField::encode(reg.code_);
 }
 
 
-void Result::CopyTo(Result* destination) const {
-  destination->static_type_ = static_type_;
-  destination->type_ = type();
-  destination->cgen_ = cgen_;
-
-  if (is_register()) {
-    destination->data_.reg_ = reg();
-    cgen_->allocator()->Use(reg());
-  } else if (is_constant()) {
-    destination->data_.handle_ = data_.handle_;
-  } else {
-    ASSERT(!is_valid());
-  }
+Result::Result(Register reg, StaticType type) {
+  ASSERT(reg.is_valid());
+  CodeGeneratorScope::Current()->allocator()->Use(reg);
+  value_ = StaticTypeField::encode(type.static_type_)
+      | TypeField::encode(REGISTER)
+      | DataField::encode(reg.code_);
 }
 
 
@@ -80,9 +63,9 @@ Result RegisterAllocator::AllocateWithoutSpilling() {
   int free_reg = registers_.ScanForFreeRegister();
   if (free_reg < kNumRegisters) {
     Register free_result = { free_reg };
-    return Result(free_result, cgen_);
+    return Result(free_result);
   }
-  return Result(cgen_);
+  return Result();
 }
 
 
@@ -94,7 +77,7 @@ Result RegisterAllocator::Allocate() {
     Register free_reg = cgen_->frame()->SpillAnyRegister();
     if (free_reg.is_valid()) {
       ASSERT(!is_used(free_reg));
-      return Result(free_reg, cgen_);
+      return Result(free_reg);
     }
   }
   return result;
@@ -104,7 +87,7 @@ Result RegisterAllocator::Allocate() {
 Result RegisterAllocator::Allocate(Register target) {
   // If the target is not referenced, it can simply be allocated.
   if (!is_used(target)) {
-    return Result(target, cgen_);
+    return Result(target);
   }
   // If the target is only referenced in the frame, it can be spilled and
   // then allocated.
@@ -112,10 +95,10 @@ Result RegisterAllocator::Allocate(Register target) {
   if (cgen_->frame()->is_used(target) && count(target) == 1)  {
     cgen_->frame()->Spill(target);
     ASSERT(!is_used(target));
-    return Result(target, cgen_);
+    return Result(target);
   }
   // Otherwise (if it's referenced outside the frame) we cannot allocate it.
-  return Result(cgen_);
+  return Result();
 }
 
 

src/register-allocator.h

@@ -103,6 +103,7 @@ class StaticType BASE_EMBEDDED {
   StaticTypeEnum static_type_;
 
   friend class FrameElement;
+  friend class Result;
 };
 
 
@@ -121,26 +122,20 @@ class Result BASE_EMBEDDED {
   };
 
   // Construct an invalid result.
-  explicit Result(CodeGenerator* cgen)
-      : static_type_(),
-        type_(INVALID),
-        cgen_(cgen) {}
+  Result() { invalidate(); }
 
   // Construct a register Result.
-  Result(Register reg,
-         CodeGenerator* cgen);
+  explicit Result(Register reg);
 
   // Construct a register Result with a known static type.
-  Result(Register reg,
-         CodeGenerator* cgen,
-         StaticType static_type);
+  Result(Register reg, StaticType static_type);
 
   // Construct a Result whose value is a compile-time constant.
-  Result(Handle<Object> value, CodeGenerator * cgen)
-      : static_type_(StaticType::TypeOf(*value)),
-        type_(CONSTANT),
-        cgen_(cgen) {
-    data_.handle_ = value.location();
+  explicit Result(Handle<Object> value) {
+    value_ = StaticTypeField::encode(StaticType::TypeOf(*value).static_type_)
+        | TypeField::encode(CONSTANT)
+        | DataField::encode(ConstantList()->length());
+    ConstantList()->Add(value);
   }
 
   // The copy constructor and assignment operators could each create a new
@@ -159,25 +154,51 @@ class Result BASE_EMBEDDED {
 
   inline ~Result();
 
+  // Static indirection table for handles to constants.  If a Result
+  // represents a constant, the data contains an index into this table
+  // of handles to the actual constants.
+  typedef ZoneList<Handle<Object> > ZoneObjectList;
+
+  static ZoneObjectList* ConstantList() {
+    static ZoneObjectList list(10);
+    return &list;
+  }
+
+  // Clear the constants indirection table.
+  static void ClearConstantList() {
+    ConstantList()->Clear();
+  }
+
   inline void Unuse();
 
-  StaticType static_type() const { return static_type_; }
-  void set_static_type(StaticType static_type) { static_type_ = static_type; }
+  StaticType static_type() const {
+    return StaticType(StaticTypeField::decode(value_));
+  }
+
+  void set_static_type(StaticType type) {
+    value_ = value_ & ~StaticTypeField::mask();
+    value_ = value_ | StaticTypeField::encode(type.static_type_);
+  }
 
-  Type type() const { return static_cast<Type>(type_); }
+  Type type() const { return TypeField::decode(value_); }
+
+  void invalidate() { value_ = TypeField::encode(INVALID); }
 
   bool is_valid() const { return type() != INVALID; }
   bool is_register() const { return type() == REGISTER; }
   bool is_constant() const { return type() == CONSTANT; }
 
   Register reg() const {
-    ASSERT(type() == REGISTER);
-    return data_.reg_;
+    ASSERT(is_register());
+    uint32_t reg = DataField::decode(value_);
+    Register result;
+    result.code_ = reg;
+    return result;
   }
 
   Handle<Object> handle() const {
     ASSERT(type() == CONSTANT);
-    return Handle<Object>(data_.handle_);
+    return ConstantList()->at(DataField::decode(value_));
   }
 
   // Move this result to an arbitrary register.  The register is not
@@ -191,17 +212,15 @@ class Result BASE_EMBEDDED {
   void ToRegister(Register reg);
 
  private:
-  StaticType static_type_;
-  byte type_;
+  uint32_t value_;
 
-  union {
-    Register reg_;
-    Object** handle_;
-  } data_;
+  class StaticTypeField: public BitField<StaticType::StaticTypeEnum, 0, 3> {};
+  class TypeField: public BitField<Type, 3, 2> {};
+  class DataField: public BitField<uint32_t, 5, 32 - 6> {};
 
-  CodeGenerator* cgen_;
+  inline void CopyTo(Result* destination) const;
 
-  void CopyTo(Result* destination) const;
+  friend class CodeGeneratorScope;
 };
 
 

src/virtual-frame.cc

@@ -324,11 +324,11 @@ void VirtualFrame::SetElementAt(int index, Result* value) {
 
   // Early exit if the element is the same as the one being set.
   bool same_register = original.is_register()
-                    && value->is_register()
-                    && original.reg().is(value->reg());
+      && value->is_register()
+      && original.reg().is(value->reg());
   bool same_constant = original.is_constant()
-                    && value->is_constant()
-                    && original.handle().is_identical_to(value->handle());
+      && value->is_constant()
+      && original.handle().is_identical_to(value->handle());
   if (same_register || same_constant) {
     value->Unuse();
     return;

test/cctest/test-log-ia32.cc

@@ -13,6 +13,7 @@
 #include "top.h"
 #include "cctest.h"
 #include "disassembler.h"
+#include "register-allocator-inl.h"
 
 using v8::Function;
 using v8::Local;