Bypass an expensive computation of a basic block's entry frame for a...

src/ia32/jump-target-ia32.cc

Index: src/ia32/jump-target-ia32.cc
===================================================================
--- src/ia32/jump-target-ia32.cc	(revision 1953)
+++ src/ia32/jump-target-ia32.cc	(working copy)
@@ -195,12 +195,12 @@
     return;
   }
 
-  if (direction_ == FORWARD_ONLY) {
-    // A simple case: no forward jumps and no possible backward jumps.
-    if (!is_linked()) {
+  if (!is_linked()) {
+    ASSERT(cgen_->has_valid_frame());
+    if (direction_ == FORWARD_ONLY) {
+      // Fast case: no forward jumps and no possible backward jumps.
       // The stack pointer can be floating above the top of the
       // virtual frame before the bind.  Afterward, it should not.
-      ASSERT(cgen_->has_valid_frame());
       VirtualFrame* frame = cgen_->frame();
       int difference =
           frame->stack_pointer_ - (frame->elements_.length() - 1);
@@ -209,34 +209,44 @@
         __ add(Operand(esp), Immediate(difference * kPointerSize));
       }
 
-      is_bound_ = true;
-      return;
+    } else {
+      ASSERT(direction_ == BIDIRECTIONAL);
+      // Fast case: no forward jumps, possible backward ones.  Remove
+      // constants and copies above the watermark on the fall-through
+      // frame and use it as the entry frame.
+      cgen_->frame()->MakeMergable(mergable_elements);
+      entry_frame_ = new VirtualFrame(cgen_->frame());
+      __ bind(&entry_label_);
     }
+    is_bound_ = true;
+    return;
+  }
 
-    // Another simple case: no fall through, a single forward jump,
-    // and no possible backward jumps.
-    if (!cgen_->has_valid_frame() && reaching_frames_.length() == 1) {
-      // Pick up the only reaching frame, take ownership of it, and
-      // use it for the block about to be emitted.
-      VirtualFrame* frame = reaching_frames_[0];
-      RegisterFile reserved = RegisterAllocator::Reserved();
-      cgen_->SetFrame(frame, &reserved);
-      reaching_frames_[0] = NULL;
-      __ bind(&merge_labels_[0]);
 

[Kasper Lund, 2009/05/15 06:26:51]

Too much spacing?

[Kevin Millikin (Chromium), 2009/05/15 08:03:35]

Done.

-      // The stack pointer can be floating above the top of the
-      // virtual frame before the bind.  Afterward, it should not.
-      int difference =
-          frame->stack_pointer_ - (frame->elements_.length() - 1);
-      if (difference > 0) {
-        frame->stack_pointer_ -= difference;
-        __ add(Operand(esp), Immediate(difference * kPointerSize));
-      }
+  if (direction_ == FORWARD_ONLY &&
+      !cgen_->has_valid_frame() &&
+      reaching_frames_.length() == 1) {

[William Hesse, 2009/05/15 07:45:51]

Can't this case be reduced to the previous case (1 reaching frame, no linked
frames) using the code below, for both cases, forward and backward links?  Then
the lines below making cgen->frame frames_[0], and dropping frames_[0], could be
before line 198, and reuse that code.  Would that be better?

[Kevin Millikin (Chromium), 2009/05/15 08:03:35]

Of course.  I planned on doing that as another change.

+    // Fast case: no fall-through, a single forward jump, and no
+    // possible backward jumps.  Pick up the only reaching frame, take
+    // ownership of it, and use it for the block about to be emitted.
+    VirtualFrame* frame = reaching_frames_[0];
+    RegisterFile reserved = RegisterAllocator::Reserved();
+    cgen_->SetFrame(frame, &reserved);
+    reaching_frames_[0] = NULL;
+    __ bind(&merge_labels_[0]);
 
-      is_linked_ = false;
-      is_bound_ = true;
-      return;
+    // The stack pointer can be floating above the top of the
+    // virtual frame before the bind.  Afterward, it should not.
+    int difference =
+        frame->stack_pointer_ - (frame->elements_.length() - 1);
+    if (difference > 0) {
+      frame->stack_pointer_ -= difference;
+      __ add(Operand(esp), Immediate(difference * kPointerSize));
     }
+
+    is_linked_ = false;
+    is_bound_ = true;
+    return;
   }
 
   // If there is a current frame, record it as the fall-through.  It
@@ -250,9 +260,7 @@
   }
 
   // Compute the frame to use for entry to the block.
-  if (entry_frame_ == NULL) {
-    ComputeEntryFrame(mergable_elements);
-  }
+  ComputeEntryFrame(mergable_elements);
 
   // Some moves required to merge to an expected frame require purely
   // frame state changes, and do not require any code generation.