Experimental: for forward CFG edges, generate the code to merge to an...

src/jump-target.h

Index: src/jump-target.h
===================================================================
--- src/jump-target.h	(revision 1075)
+++ src/jump-target.h	(working copy)
@@ -35,6 +35,8 @@
 // -------------------------------------------------------------------------
 // Jump targets
 //
+// TODO(): Update this comment.

[Kasper Lund, 2009/01/15 10:15:02]

You should probably just do this.

[Kevin Millikin (Chromium), 2009/01/15 13:08:18]

Done.

+//
 // A jump target is an abstraction of a control-flow target in generated
 // code.  It encapsulates an assembler label and an expected virtual frame
 // layout at that label.  The first time control flow reaches the target,
@@ -59,7 +61,7 @@
   // useful, eg, when jump targets are embedded in AST nodes.
   JumpTarget();
 
-  virtual ~JumpTarget() { delete expected_frame_; }
+  virtual ~JumpTarget() { Unuse(); }
 
   // Supply a code generator.  This function expects to be given a non-null
   // code generator, and to be called only when the code generator is not
@@ -69,7 +71,7 @@
   // Accessors.
   CodeGenerator* code_generator() const { return cgen_; }
 
-  Label* label() { return &label_; }
+  Label* entry_label() { return &entry_label_; }
 
   VirtualFrame* expected_frame() const { return expected_frame_; }
   void set_expected_frame(VirtualFrame* frame) {
@@ -77,53 +79,92 @@
   }
 
   // Predicates testing the state of the encapsulated label.
-  bool is_bound() const { return label_.is_bound(); }
-  bool is_linked() const { return label_.is_linked(); }
-  bool is_unused() const { return label_.is_unused(); }
+  bool is_bound() const { return expected_frame_ != NULL; }
+  bool is_linked() const { return reaching_frames_.length() > 0; }
+  bool is_unused() const { return !is_bound() && !is_linked(); }
 
-  // Treat the jump target as a fresh one---the label is unused and the
-  // expected frame if any is reset.
+  // Treat the jump target as a fresh one.  The expected frame if any
+  // will be deallocated and there should be no dangling jumps to the
+  // target (thus no reaching frames).
   void Unuse() {
-    label_.Unuse();
+    ASSERT(!is_linked());
+    entry_label_.Unuse();
     delete expected_frame_;
     expected_frame_ = NULL;
   }
 
-  // Emit a jump to the target.  There must be a current frame before the
+  // Reset the internal state of this jump target.  Pointed-to virtual
+  // frames are not deallocated and dangling jumps to the target are
+  // left dangling.
+  void Reset() {
+    reaching_frames_.Clear();
+    merge_labels_.Clear();
+    expected_frame_ = NULL;

[Kasper Lund, 2009/01/15 10:15:02]

Do we need to delete the expected frame here before setting it to NULL? If not,
it should probably be mentioned in a comment.

[Kevin Millikin (Chromium), 2009/01/15 13:08:18]

We don't want to delete it because it has normally been copied.  I think the
comment here before the function is pretty clear.

+    entry_label_.Unuse();
+  }
+
+  // Copy the state of this jump target to the destination.  The lists
+  // of forward-reaching frames and merge-point labels are copied.
+  // All virtual frame pointers are copied, not the pointed-to frames.
+  // The previous state of the destination is overwritten, without
+  // deallocating pointed-to virtual frames.
+  void CopyTo(JumpTarget* destination);
+
+  // Emit a jump to the target.  There must be a current frame at the
   // jump and there will be no current frame after the jump.
   void Jump();
   void Jump(Result* arg);
 
-  // Emit a conditional branch to the target.  If there is no current frame,
-  // there must be one expected at the target.
+  // Emit a conditional branch to the target.  There must be a current
+  // frame at the branch.  The current frame will fall through to the
+  // code after the branch.
   void Branch(Condition cc, Hint hint = no_hint);
   void Branch(Condition cc, Result* arg, Hint hint = no_hint);
   void Branch(Condition cc, Result* arg0, Result* arg1, Hint hint = no_hint);
 
-  // Bind a jump target.  There must be a current frame and no expected
-  // frame at the target (targets are only bound once).
+  // Bind a jump target.  If there is no current frame at the binding
+  // site, there must be an expected one at the target, which will
+  // become the current frame after the bind.

[William Hesse, 2009/01/15 12:21:54]

Are we allowing non-mergable expected frames, which might be changed to be
mergable or merged when bound, so that the frame after the binding site won't be
exactly the expected frame?

[Kevin Millikin (Chromium), 2009/01/15 13:08:18]

The frame after the binding site will always be the expected one.  I've just
changed the comment to be more specific.

   void Bind();
   void Bind(Result* arg);
   void Bind(Result* arg0, Result* arg1);
 
-  // Emit a call to a jump target.  There must be a current frame.  The
-  // frame at the target is the same as the current frame except for an
-  // extra return address on top of it.
+  // Emit a call to a jump target.  There must be a current frame at
+  // the call.  The frame at the target is the same as the current
+  // frame except for an extra return address on top of it.  The frame
+  // after the call is the same as the frame before the call.
   void Call();
 
  protected:
-  // The encapsulated assembler label.
-  Label label_;
+  // The code generator gives access to its current frame.
+  CodeGenerator* cgen_;
 
-  // The expected frame where the label is bound, or NULL.
-  VirtualFrame* expected_frame_;
+  // Used to emit code.
+  MacroAssembler* masm_;
 
  private:
-  // The code generator gives access to the current frame.
-  CodeGenerator* cgen_;
+  // A list of frames reaching this block via forward jumps.
+  List<VirtualFrame*> reaching_frames_;
 
-  // Used to emit code.
-  MacroAssembler* masm_;
+  // A parallel list of labels for merge code.
+  List<Label> merge_labels_;
+
+  // The (mergable) frame expected at backward jumps to the block.
+  VirtualFrame* expected_frame_;
+
+  // The actual entry label of the block.
+  Label entry_label_;
+
+  // Add a virtual frame reaching this labeled block via a forward
+  // jump, and a fresh label for its merge code.
+  void AddReachingFrame(VirtualFrame* frame) {

[Kasper Lund, 2009/01/15 10:15:02]

I don't know if this really should be inline. Maybe it would be better to move
it to the .cc file?

[Kevin Millikin (Chromium), 2009/01/15 13:08:18]

Done.

+    ASSERT(reaching_frames_.length() == merge_labels_.length());
+    Label fresh;
+    merge_labels_.Add(fresh);
+    reaching_frames_.Add(frame);
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(JumpTarget);
 };
 
 
@@ -139,10 +180,10 @@
 
 class ShadowTarget : public JumpTarget {
  public:
-  // Construct a shadow a jump target.  After construction, the original
-  // jump target shadows the former target, which is hidden as the
+  // Construct a shadow jump target.  After construction, the original
+  // jump target shadows other one, which is hidden as the

[William Hesse, 2009/01/15 12:21:54]

Not grammatical, confusing.

[Kevin Millikin (Chromium), 2009/01/15 13:08:18]

Added "the".

   // newly-constructed shadow target.
-  explicit ShadowTarget(JumpTarget* original);
+  explicit ShadowTarget(JumpTarget* shadowed);
 
   virtual ~ShadowTarget() {
     ASSERT(!is_shadowing_);
@@ -155,20 +196,13 @@
 
   // During shadowing, the currently shadowing target.  After shadowing, the
   // target that was shadowed.
-  JumpTarget* original_target() const { return original_target_; }
+  JumpTarget* other_target() const { return other_target_; }

[William Hesse, 2009/01/15 12:21:54]

Could this be called "active" target?  This is the JumpTarget passed to the
constructor, which always seems to be the active one, or the one that is "seen".
 The ShadowTarget represents the hidden target while it is masked by another,
and the target that was used to hide the original, after the hiding is over,
right?

[Kevin Millikin (Chromium), 2009/01/15 13:08:18]

No.  After shadowing they are both active.  I can't think of any better name for
this thing that is accurate for its whole life.  It is simply the "other"
target, the one that is not represented by the shadow target.

 
  private:
   // During shadowing, the currently shadowing target.  After shadowing, the
   // target that was shadowed.
-  JumpTarget* original_target_;
+  JumpTarget* other_target_;
 
-  // During shadowing, the saved state of the shadowed target's label.
-  int original_pos_;
-
-  // During shadowing, the saved state of the shadowed target's expected
-  // frame.
-  VirtualFrame* original_expected_frame_;
-
 #ifdef DEBUG
   bool is_shadowing_;
 #endif