Change the way we handle backward jumps in the code generator. Keep...

src/jump-target.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 53 matching lines @@
   ASSERT(cgen != NULL);
   ASSERT(cgen_ == NULL);
   cgen_ = cgen;
   masm_ = cgen->masm();
   direction_ = direction;
 }
 
 
 void JumpTarget::Unuse() {
   ASSERT(!is_linked());
-#ifdef DEBUG
   for (int i = 0; i < reaching_frames_.length(); i++) {
-    ASSERT(reaching_frames_[i] == NULL);
+    delete reaching_frames_[i];

[Kasper Lund, 2009/03/12 14:45:00]

Do you want to NULL them out too in case the JumpTarget is re-used?

[Kevin Millikin (Chromium), 2009/03/12 14:53:51]

Maybe it's safer (in the face of changes).  Right now, the entire list is
cleared in the call to Reset.

   }
-#endif
   delete entry_frame_;
-
   Reset();
 }
 
 
 void JumpTarget::Reset() {
   reaching_frames_.Clear();
   merge_labels_.Clear();
   entry_frame_ = NULL;
   entry_label_.Unuse();
   is_bound_ = false;
@@ skipping 45 matching lines @@
       return right;
     }
   }
 
   // Otherwise they are incompatible and we will reallocate them.
   return NULL;
 }
 
 
 void JumpTarget::ComputeEntryFrame(int mergable_elements) {
-  // Given: a collection of frames reaching by forward CFG edges
-  // (including the code generator's current frame) and the
-  // directionality of the block.  Compute: an entry frame for the
+  // Given: a collection of frames reaching by forward CFG edges and
+  // the directionality of the block.  Compute: an entry frame for the
   // block.
 
-  // Choose an initial frame, either the code generator's current
-  // frame if there is one, or the first reaching frame if not.
-  VirtualFrame* initial_frame = cgen_->frame();
-  int start_index = 0;  // Begin iteration with the 1st reaching frame.
-  if (initial_frame == NULL) {
-    initial_frame = reaching_frames_[0];
-    start_index = 1;  // Begin iteration with the 2nd reaching frame.
-  }
+  // Choose an initial frame.
+  VirtualFrame* initial_frame = reaching_frames_[0];
 
   // A list of pointers to frame elements in the entry frame.  NULL
   // indicates that the element has not yet been determined.
   int length = initial_frame->elements_.length();
   List<FrameElement*> elements(length);
 
   // Convert the number of mergable elements (counted from the top
   // down) to a frame high-water mark (counted from the bottom up).
   // Elements strictly above the high-water index will be mergable in
   // entry frames for bidirectional jump targets.
   int high_water_mark = (mergable_elements == kAllElements)
       ? VirtualFrame::kIllegalIndex  // All frame indices are above this.
       : length - mergable_elements - 1;  // Top index if m_e == 0.
 
   // Initially populate the list of elements based on the initial
   // frame.
   for (int i = 0; i < length; i++) {
     FrameElement element = initial_frame->elements_[i];
     // We do not allow copies or constants in bidirectional frames.
     if (direction_ == BIDIRECTIONAL &&
         i > high_water_mark &&
         (element.is_constant() || element.is_copy())) {
       elements.Add(NULL);
     } else {
       elements.Add(&initial_frame->elements_[i]);
     }
   }
 
   // Compute elements based on the other reaching frames.
-  if (start_index < reaching_frames_.length()) {
+  if (reaching_frames_.length() > 1) {
     for (int i = 0; i < length; i++) {
-      for (int j = start_index; j < reaching_frames_.length(); j++) {
+      for (int j = 1; j < reaching_frames_.length(); j++) {
         FrameElement* element = elements[i];
 
         // Element computation is monotonic: new information will not
         // change our decision about undetermined or invalid elements.
         if (element == NULL || !element->is_valid()) break;
 
         elements[i] = Combine(element, &reaching_frames_[j]->elements_[i]);
       }
     }
   }
@@ skipping 18 matching lines @@
 
 
   // Copy the already-determined frame elements to the entry frame,
   // and allocate any still-undetermined frame elements to registers
   // or memory, from the top down.
   for (int i = length - 1; i >= 0; i--) {
     if (elements[i] == NULL) {
       // If the value is synced on all frames, put it in memory.  This
       // costs nothing at the merge code but will incur a
       // memory-to-register move when the value is needed later.
-      bool is_synced = initial_frame->elements_[i].is_synced();
-      int j = start_index;
-      while (is_synced && j < reaching_frames_.length()) {
+      bool is_synced = true;
+      for (int j = 0; is_synced && j < reaching_frames_.length(); j++) {
         is_synced = reaching_frames_[j]->elements_[i].is_synced();
-        j++;
       }
+
       // There is nothing to be done if the elements are all synced.
       // It is already recorded as a memory element.
       if (is_synced) continue;
 
       // Choose an available register.  Prefer ones that the element
       // is already occupying on some reaching frame.
       RegisterFile candidate_registers;
       int max_count = kMinInt;
       int best_reg_code = no_reg.code_;
 
-      // Consider the initial frame.
-      FrameElement element = initial_frame->elements_[i];
-      if (element.is_register() &&
-          !frame_registers.is_used(element.reg())) {
-        candidate_registers.Use(element.reg());
-        max_count = 1;
-        best_reg_code = element.reg().code();
-      }
-      // Consider the other frames.
-      for (int j = start_index; j < reaching_frames_.length(); j++) {
-        element = reaching_frames_[j]->elements_[i];
+      for (int j = 0; j < reaching_frames_.length(); j++) {
+        FrameElement element = reaching_frames_[j]->elements_[i];
         if (element.is_register() &&
             !frame_registers.is_used(element.reg())) {
           candidate_registers.Use(element.reg());
           if (candidate_registers.count(element.reg()) > max_count) {
             max_count = candidate_registers.count(element.reg());
             best_reg_code = element.reg().code();
           }
         }
       }
       // If there was no preferred choice consider any free register.
@@ skipping 316 matching lines @@
   } else {
     JumpTarget::Branch(cc, hint);
   }
 }
 
 
 void BreakTarget::Bind(int mergable_elements) {
 #ifdef DEBUG
   ASSERT(mergable_elements == kAllElements);
   ASSERT(cgen_ != NULL);
+  // All the forward-reaching frames should have been adjusted at the
+  // jumps to this target.
   for (int i = 0; i < reaching_frames_.length(); i++) {
-    ASSERT(reaching_frames_[i]->height() == expected_height_);
+    ASSERT(reaching_frames_[i] == NULL ||
+           reaching_frames_[i]->height() == expected_height_);
   }
 #endif
-
-  // This is a break target so drop leftover statement state from the
-  // frame before merging.
+  // This is a break target so we drop leftover statement state from
+  // the frame before merging, even on the fall through.  This is
+  // because we can bind the return target with state on the frame.
   if (cgen_->has_valid_frame()) {
-    int count = cgen_->frame()->height() - expected_height_;
-    cgen_->frame()->ForgetElements(count);
+    cgen_->frame()->ForgetElements(cgen_->frame()->height() - expected_height_);
   }
   JumpTarget::Bind(mergable_elements);
 }
 
 
 // -------------------------------------------------------------------------
 // ShadowTarget implementation.
 
 ShadowTarget::ShadowTarget(BreakTarget* shadowed) {
   ASSERT(shadowed != NULL);
@@ skipping 36 matching lines @@
   temp.CopyTo(this);
   temp.Reset();  // So the destructor does not deallocate virtual frames.
 
 #ifdef DEBUG
   is_shadowing_ = false;
 #endif
 }
 
 
 } }  // namespace v8::internal