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

src/ia32/jump-target-ia32.cc

 // Copyright 2008 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 177 matching lines @@
       // Take ownership of the entry frame.
       cgen_->SetFrame(entry_frame_, &reserved);
       entry_frame_ = NULL;
     }
     __ bind(&entry_label_);
     is_linked_ = false;
     is_bound_ = true;
     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);
       if (difference > 0) {
         frame->stack_pointer_ -= difference;
         __ 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;
+  }
+
+

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

Too much spacing?

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

Done.

+  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]);
+
+    // 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));
     }
 
-    // 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]);
-
-      // 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;
-    }
+    is_linked_ = false;
+    is_bound_ = true;
+    return;
   }
 
   // If there is a current frame, record it as the fall-through.  It
   // is owned by the reaching frames for now.
   bool had_fall_through = false;
   if (cgen_->has_valid_frame()) {
     had_fall_through = true;
     AddReachingFrame(cgen_->frame());  // Return value ignored.
     RegisterFile empty;
     cgen_->SetFrame(NULL, &empty);
   }
 
   // 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.
   // Perform those first to increase the possibility of finding equal
   // frames below.
   for (int i = 0; i < reaching_frames_.length(); i++) {
     if (reaching_frames_[i] != NULL) {
       reaching_frames_[i]->PrepareMergeTo(entry_frame_);
     }
   }
@@ skipping 99 matching lines @@
   }
 
   is_linked_ = false;
   is_bound_ = true;
 }
 
 #undef __
 
 
 } }  // namespace v8::internal