Experimental: initial simple support for registers in the virtual...

v8/src/virtual-frame-ia32.h

Index: v8/src/virtual-frame-ia32.h
===================================================================
--- v8/src/virtual-frame-ia32.h	(revision 792)
+++ v8/src/virtual-frame-ia32.h	(working copy)
@@ -41,16 +41,18 @@
 
 class FrameElement BASE_EMBEDDED {
  public:
-  enum Type { MEMORY, CONSTANT, LAST_TYPE = CONSTANT };
+  enum Type { MEMORY, REGISTER, CONSTANT, LAST_TYPE = CONSTANT };
 
   FrameElement() : type_(MEMORY) {}
 
+  explicit FrameElement(Register reg) : type_(REGISTER | kDirtyBit) {
+    data_.reg_ = reg;
+  }
+
   explicit FrameElement(Handle<Object> value) : type_(CONSTANT | kDirtyBit) {
     data_.handle_ = value.location();
   }
 
-  Type type() const { return static_cast<Type>(type_ & kTypeMask); }
-
   bool is_dirty() const {
     return (type_ & kDirtyBit) != 0;
   }
@@ -65,6 +67,15 @@
     type_ = type_ & ~kDirtyBit;
   }
 
+  bool is_spilled() const { return type() == MEMORY; }
+  bool is_register() const { return type() == REGISTER; }
+  bool is_constant() const { return type() == CONSTANT; }

[William Hesse, 2008/11/19 15:05:36]

If is_spilled() is only true when type is MEMORY, then
spilling a constant by marking it non-dirty won't be enough - we will have to
forget the fact that it is a constant.
But maybe that is what you mean by spilling - it is needed for
merging, but not for branching or function calls.
Will we have different terms for "everthing non-dirty, all registers free" and
"everything non-dirty, no info at all"?

[iposva, 2008/11/20 05:49:35]

Should there be an is_synched() accessor to make the distinction clearer to the
reader. I am not sure though if it would be useful in the code.

[Kevin Millikin (Chromium), 2008/11/20 08:28:03]

Maybe it's best to rename is_spilled to something more obvious.  is_in_memory
seems accurate but I don't like reading it.

+
+  Register reg() const {
+    ASSERT(type() == REGISTER);
+    return data_.reg_;
+  }
+
   Handle<Object> handle() const {
     ASSERT(type() == CONSTANT);
     return Handle<Object>(data_.handle_);
@@ -74,7 +85,7 @@
   static const int kDirtyBit = 1 << 8;

[iposva, 2008/11/20 05:49:35]

As I said in my other comments. You probably really want to use the BitField
template here.

   static const int kTypeMask = kDirtyBit - 1;
 
-  STATIC_ASSERT((kDirtyBit > LAST_TYPE));
+  STATIC_ASSERT(kDirtyBit > LAST_TYPE);
 
   // The element's type and a dirty bit.  The dirty bit can be cleared
   // for non-memory elements to indicate that the element agrees with
@@ -82,8 +93,11 @@
   int type_;
 
   union {
+    Register reg_;
     Object** handle_;
   } data_;
+
+  Type type() const { return static_cast<Type>(type_ & kTypeMask); }

[William Hesse, 2008/11/19 15:05:36]

Will code constructors only be able to access the type of "Result", to make
decisions on?  That makes sense, I guess.

[Kevin Millikin (Chromium), 2008/11/20 08:28:03]

I decided not to expose the type (in fact, the enum should probably be private
too).  Internally, we might have a lot of different "types" for administrative
purposes (eg, constant vs. source constant) that the clients of the frame don't
care about.

The place where types might be better than predicates is if clients want to
write a switch instead of nested if-then-else.  But that code is brittle to
addition of types anyway....

 };
 
 
@@ -259,6 +273,18 @@
     return (frame_pointer_ - index) * kPointerSize;
   }
 
+  // Sync the element at a particular index---write it to memory if
+  // necessary, but do not free any associated register or forget its value
+  // if constant.  Space should have already been allocated in the actual
+  // frame for all the elements below this one (at least).

[iposva, 2008/11/20 05:49:35]

Not just the space should be allocated, but all the elements should be on the
stack at least at GC points.

[Kevin Millikin (Chromium), 2008/11/20 08:28:03]

I agree that "sync" vs. "spill" is fuzzy.  The point is that we sometimes want
to allocate space in the frame to spill a value (in case we later need to spill
it), without actually spilling it (eg, without forgetting that it is a constant
or in a particular register).

But, as you say, we have to put something safe for GC in that slot so it makes
sense to put the real value there and avoid a move later in the event of a spill
before the frame element is changed again.

Thus "sync"---to make the value in memory agree with the frame but not change
the frame value.

It will likely only be used for allocating space for a spill, so changing the
name probably removes confusion (I just can't think of a better name right now).

+  void SyncElementAt(int index);
+
+  // Spill the element at a particular index---write it to memory if
+  // necessary, free any associated register, and forget its value if
+  // constant.  Space should have already been allocated in the actual frame
+  // for all the elements below this one (at least).
+  void SpillElementAt(int index);

[William Hesse, 2008/11/19 15:05:36]

OK, this answers my question about different terms for spill and sync.

+
   // Spill the topmost elements of the frame to memory (eg, they are the
   // arguments to a call) and all registers.
   void PrepareForCall(int count);