ARM: Change code generation for function return

Generating code for function return on ARM is now aligned with the other platforms. The first non-shadowed return statement encountered will emit code for function return and all other returns including the fall through at the bottom of a function will jump to that.
Review URL: http://codereview.chromium.org/2815003

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4856 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/arm/codegen-arm.cc b/src/arm/codegen-arm.cc
index daa7870a0c1598b5853752079bea879375fec53e..325e67abec390c85506c2e791308fdb42442e777 100644 (file)
--- a/src/arm/codegen-arm.cc
+++ b/src/arm/codegen-arm.cc
@@ -342,56 +342,27 @@ void CodeGenerator::Generate(CompilationInfo* info) {
     }
   }
 
-  // Generate the return sequence if necessary.
-  if (has_valid_frame() || function_return_.is_linked()) {
-    if (!function_return_.is_linked()) {
-      CodeForReturnPosition(info->function());
-    }
-    // exit
-    // r0: result
-    // sp: stack pointer
-    // fp: frame pointer
-    // cp: callee's context
+  // Handle the return from the function.
+  if (has_valid_frame()) {
+    // If there is a valid frame, control flow can fall off the end of
+    // the body.  In that case there is an implicit return statement.
+    ASSERT(!function_return_is_shadowed_);
+    frame_->PrepareForReturn();
     __ LoadRoot(r0, Heap::kUndefinedValueRootIndex);
-
+    if (function_return_.is_bound()) {
+      function_return_.Jump();
+    } else {
+      function_return_.Bind();
+      GenerateReturnSequence();
+    }
+  } else if (function_return_.is_linked()) {
+    // If the return target has dangling jumps to it, then we have not
+    // yet generated the return sequence.  This can happen when (a)
+    // control does not flow off the end of the body so we did not
+    // compile an artificial return statement just above, and (b) there
+    // are return statements in the body but (c) they are all shadowed.
     function_return_.Bind();
-    if (FLAG_trace) {
-      // Push the return value on the stack as the parameter.
-      // Runtime::TraceExit returns the parameter as it is.
-      frame_->EmitPush(r0);
-      frame_->CallRuntime(Runtime::kTraceExit, 1);
-    }
-
-#ifdef DEBUG
-    // Add a label for checking the size of the code used for returning.
-    Label check_exit_codesize;
-    masm_->bind(&check_exit_codesize);
-#endif
-    // Make sure that the constant pool is not emitted inside of the return
-    // sequence.
-    { Assembler::BlockConstPoolScope block_const_pool(masm_);
-      // Tear down the frame which will restore the caller's frame pointer and
-      // the link register.
-      frame_->Exit();
-
-      // Here we use masm_-> instead of the __ macro to avoid the code coverage
-      // tool from instrumenting as we rely on the code size here.
-      int32_t sp_delta = (scope()->num_parameters() + 1) * kPointerSize;
-      masm_->add(sp, sp, Operand(sp_delta));
-      masm_->Jump(lr);
-
-#ifdef DEBUG
-      // Check that the size of the code used for returning matches what is
-      // expected by the debugger. If the sp_delts above cannot be encoded in
-      // the add instruction the add will generate two instructions.
-      int return_sequence_length =
-          masm_->InstructionsGeneratedSince(&check_exit_codesize);
-      CHECK(return_sequence_length ==
-            Assembler::kJSReturnSequenceInstructions ||
-            return_sequence_length ==
-            Assembler::kJSReturnSequenceInstructions + 1);
-#endif
-    }
+    GenerateReturnSequence();
   }
 
   // Adjust for function-level loop nesting.
@@ -1958,8 +1929,56 @@ void CodeGenerator::VisitReturnStatement(ReturnStatement* node) {
     // returning thus making it easier to merge.
     frame_->EmitPop(r0);
     frame_->PrepareForReturn();
+    if (function_return_.is_bound()) {
+      // If the function return label is already bound we reuse the
+      // code by jumping to the return site.
+      function_return_.Jump();
+    } else {
+      function_return_.Bind();
+      GenerateReturnSequence();
+    }
+  }
+}
 
-    function_return_.Jump();
+
+void CodeGenerator::GenerateReturnSequence() {
+  if (FLAG_trace) {
+    // Push the return value on the stack as the parameter.
+    // Runtime::TraceExit returns the parameter as it is.
+    frame_->EmitPush(r0);
+    frame_->CallRuntime(Runtime::kTraceExit, 1);
+  }
+
+#ifdef DEBUG
+  // Add a label for checking the size of the code used for returning.
+  Label check_exit_codesize;
+  masm_->bind(&check_exit_codesize);
+#endif
+  // Make sure that the constant pool is not emitted inside of the return
+  // sequence.
+  { Assembler::BlockConstPoolScope block_const_pool(masm_);
+    // Tear down the frame which will restore the caller's frame pointer and
+    // the link register.
+    frame_->Exit();
+
+    // Here we use masm_-> instead of the __ macro to avoid the code coverage
+    // tool from instrumenting as we rely on the code size here.
+    int32_t sp_delta = (scope()->num_parameters() + 1) * kPointerSize;
+    masm_->add(sp, sp, Operand(sp_delta));
+    masm_->Jump(lr);
+    DeleteFrame();
+
+#ifdef DEBUG
+    // Check that the size of the code used for returning matches what is
+    // expected by the debugger. If the sp_delts above cannot be encoded in
+    // the add instruction the add will generate two instructions.
+    int return_sequence_length =
+        masm_->InstructionsGeneratedSince(&check_exit_codesize);
+    CHECK(return_sequence_length ==
+          Assembler::kJSReturnSequenceInstructions ||
+          return_sequence_length ==
+          Assembler::kJSReturnSequenceInstructions + 1);
+#endif
   }
 }
 

diff --git a/src/arm/codegen-arm.h b/src/arm/codegen-arm.h
index 91adff0f66fc51c16ae41586c776ac5e8439e66e..eeb89e071298f5721daa86785cbf8cfd2f2362a1 100644 (file)
--- a/src/arm/codegen-arm.h
+++ b/src/arm/codegen-arm.h
@@ -315,6 +315,12 @@ class CodeGenerator: public AstVisitor {
   // Main code generation function
   void Generate(CompilationInfo* info);
 
+  // Generate the return sequence code.  Should be called no more than
+  // once per compiled function, immediately after binding the return
+  // target (which can not be done more than once).  The return value should
+  // be in r0.
+  void GenerateReturnSequence();
+
   // Returns the arguments allocation mode.
   ArgumentsAllocationMode ArgumentsMode();
 

diff --git a/src/arm/virtual-frame-arm.h b/src/arm/virtual-frame-arm.h
index e97ad49649ad91e0841f2d4e2612072167145869..d8dc5c6c05c3034a7e51bbe6c00a8ca3503d1b07 100644 (file)
--- a/src/arm/virtual-frame-arm.h
+++ b/src/arm/virtual-frame-arm.h
@@ -212,10 +212,9 @@ class VirtualFrame : public ZoneObject {
   void Enter();
   void Exit();
 
-  // Prepare for returning from the frame by spilling locals and
-  // dropping all non-locals elements in the virtual frame.  This
+  // Prepare for returning from the frame by elements in the virtual frame. This
   // avoids generating unnecessary merge code when jumping to the
-  // shared return site.  Emits code for spills.
+  // shared return site. No spill code emitted. Value to return should be in r0.
   inline void PrepareForReturn();
 
   // Number of local variables after when we use a loop for allocating.

diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc
index fc2f91fa7254fa4c15698588da2613a1209cda21..c7f008b69e50539aedc67ed56bd924b7a20a5d42 100644 (file)
--- a/src/ia32/codegen-ia32.cc
+++ b/src/ia32/codegen-ia32.cc
@@ -3278,6 +3278,9 @@ void CodeGenerator::VisitAndSpill(Statement* statement) {
 
 
 void CodeGenerator::VisitStatementsAndSpill(ZoneList<Statement*>* statements) {
+#ifdef DEBUG
+  int original_height = frame_->height();
+#endif
   ASSERT(in_spilled_code());
   set_in_spilled_code(false);
   VisitStatements(statements);
@@ -3285,14 +3288,20 @@ void CodeGenerator::VisitStatementsAndSpill(ZoneList<Statement*>* statements) {
     frame_->SpillAll();
   }
   set_in_spilled_code(true);
+
+  ASSERT(!has_valid_frame() || frame_->height() == original_height);
 }
 
 
 void CodeGenerator::VisitStatements(ZoneList<Statement*>* statements) {
+#ifdef DEBUG
+  int original_height = frame_->height();
+#endif
   ASSERT(!in_spilled_code());
   for (int i = 0; has_valid_frame() && i < statements->length(); i++) {
     Visit(statements->at(i));
   }
+  ASSERT(!has_valid_frame() || frame_->height() == original_height);
 }
 
 

diff --git a/src/virtual-frame-light-inl.h b/src/virtual-frame-light-inl.h
index d08b5d2183f512bd579f71a119ac70cb782de145..19520a6349d63dfce500276da7edba2ceec989ba 100644 (file)
--- a/src/virtual-frame-light-inl.h
+++ b/src/virtual-frame-light-inl.h
@@ -74,7 +74,9 @@ bool VirtualFrame::Equals(const VirtualFrame* other) {
 
 
 void VirtualFrame::PrepareForReturn() {
-  SpillAll();
+  // Don't bother flushing tos registers as returning does not require more
+  // access to the expression stack.
+  top_of_stack_state_ = NO_TOS_REGISTERS;
 }