[turbofan] Frame elision for code stubs

src/compiler/code-generator.cc

Index: src/compiler/code-generator.cc
diff --git a/src/compiler/code-generator.cc b/src/compiler/code-generator.cc
index b091a5ceb8ffda5c142c7784197ac1e1cc2086f4..cf723cd191537d8b3dd9802c33ad0d73171d00f3 100644
--- a/src/compiler/code-generator.cc
+++ b/src/compiler/code-generator.cc
@@ -56,12 +56,8 @@ CodeGenerator::CodeGenerator(Frame* frame, Linkage* linkage,
   for (int i = 0; i < code->InstructionBlockCount(); ++i) {
     new (&labels_[i]) Label;
   }
-  if (code->ContainsCall()) {
-    frame->MarkNeedsFrame();
-  }
 }
 
-
 Handle<Code> CodeGenerator::GenerateCode() {
   CompilationInfo* info = this->info();
 
@@ -80,10 +76,6 @@ Handle<Code> CodeGenerator::GenerateCode() {
   }
   // Architecture-specific, linkage-specific prologue.
   info->set_prologue_offset(masm()->pc_offset());
-  AssemblePrologue();
-  if (linkage()->GetIncomingDescriptor()->InitializeRootRegister()) {
-    masm()->InitializeRootRegister();
-  }
 
   // Define deoptimization literals for all inlined functions.
   DCHECK_EQ(0u, deoptimization_literals_.size());
@@ -103,7 +95,9 @@ Handle<Code> CodeGenerator::GenerateCode() {
       DefineDeoptimizationLiteral(inlined.inlined_code_object_root);
     }
   }
-
+  // Finish the Frame
+  frame()->AlignFrame(kFrameAlignmentInBytes);
+  SetupStackPointer();
   // Assemble all non-deferred blocks, followed by deferred ones.
   for (int deferred = 0; deferred < 2; ++deferred) {
     for (const InstructionBlock* block : code()->instruction_blocks()) {
@@ -143,9 +137,39 @@ Handle<Code> CodeGenerator::GenerateCode() {
         SNPrintF(buffer, " --");
         masm()->RecordComment(buffer_start);
       }
+
+      frame_access_state()->MarkHasFrame(block->needs_frame());
+
       masm()->bind(GetLabel(current_block_));
-      for (int i = block->code_start(); i < block->code_end(); ++i) {
-        AssembleInstruction(code()->InstructionAt(i));
+      if (block->must_construct_frame()) {
+        AssemblePrologue();

[danno, 2016/03/24 12:53:05]

You should rename this to AssembleConstructFrame

[Mircea Trofin, 2016/03/24 17:46:11]

Addressed in first comment.

+        if (linkage()->GetIncomingDescriptor()->InitializeRootRegister()) {
+          masm()->InitializeRootRegister();
+        }
+      }
+
+      for (int i = block->code_start(); i < block->last_instruction_index();

[danno, 2016/03/24 12:53:05]

I think it would be slightly cleaner if you keep the original single loop and
check against the index in the loop, e.g.:

Instruction* last_instr =
code()->InstructionAt(block->last_instruction_index());
for (int i = block->code_start(); i < block->code_end(); ++i) {
  Instruction* instr = code()->InstructionAt(i);
  AssembleGaps(instr);
  if (instr == last_instr && block->must_deconstruct_frame() && !instr->IsRet())
{
   AssembleDeconstructFrame(instr)
  }
  AssembleInstruction();
}

+           ++i) {
+        Instruction* instr = code()->InstructionAt(i);
+        AssembleGaps(instr);
+        AssembleInstruction(instr);
+      }
+      Instruction* last_instr =
+          code()->InstructionAt(block->last_instruction_index());
+      AssembleGaps(last_instr);
+      if (block->must_deconstruct_frame()) {
+        DCHECK(!last_instr->IsBranch());
+        if (last_instr->IsJump()) {
+          AssembleDeconstructFrame();
+          AssembleInstruction(last_instr);
+        } else if (last_instr->IsRet()) {
+          AssembleInstruction(last_instr);
+        } else {
+          AssembleInstruction(last_instr);
+          AssembleDeconstructFrame();

[danno, 2016/03/24 12:53:05]

In what case does this actually happen? It seems to not be symmetric and
suggests something else is not quite right. Every block that can deconstruction
must end either in jump or a ret, everything else (switch, tail call, branch)
should never end up here, and therefore the code I suggest above should be
sufficient, right?

+        }
+      } else {
+        AssembleInstruction(last_instr);
       }
     }
   }
@@ -290,9 +314,7 @@ bool CodeGenerator::IsMaterializableFromRoot(
   return false;
 }
 
-
 void CodeGenerator::AssembleInstruction(Instruction* instr) {
-  AssembleGaps(instr);
   AssembleSourcePosition(instr);
   // Assemble architecture-specific code for the instruction.
   AssembleArchInstruction(instr);
@@ -761,19 +783,20 @@ DeoptimizationExit* CodeGenerator::AddDeoptimizationExit(
 }
 
 int CodeGenerator::TailCallFrameStackSlotDelta(int stack_param_delta) {
-  CallDescriptor* descriptor = linkage()->GetIncomingDescriptor();
-  int spill_slots = frame()->GetSpillSlotCount();
-  bool has_frame = descriptor->IsJSFunctionCall() || spill_slots > 0;
+  //  CallDescriptor* descriptor = linkage()->GetIncomingDescriptor();
+  //  int spill_slots = frame()->GetSpillSlotCount();
+  // bool has_frame = descriptor->IsJSFunctionCall() || spill_slots > 0;
+  // DCHECK(has_frame == frame_access_state()->frame_enabled());

[danno, 2016/03/24 12:53:05]

Please remove the commented-out code.

[Mircea Trofin, 2016/03/24 17:46:11]

Done.

   // Leave the PC on the stack on platforms that have that as part of their ABI
   int pc_slots = V8_TARGET_ARCH_STORES_RETURN_ADDRESS_ON_STACK ? 1 : 0;
-  int sp_slot_delta =
-      has_frame ? (frame()->GetTotalFrameSlotCount() - pc_slots) : 0;
+  int sp_slot_delta = frame_access_state()->has_frame()
+                          ? (frame()->GetTotalFrameSlotCount() - pc_slots)
+                          : 0;
   // Discard only slots that won't be used by new parameters.
   sp_slot_delta += stack_param_delta;
   return sp_slot_delta;
 }
 
-
 OutOfLineCode::OutOfLineCode(CodeGenerator* gen)
     : frame_(gen->frame()), masm_(gen->masm()), next_(gen->ools_) {
   gen->ools_ = this;