[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..b4a880fc1d6e02bc02759f27ac887f1fc26fe1ad 100644
--- a/src/compiler/code-generator.cc
+++ b/src/compiler/code-generator.cc
@@ -52,15 +52,24 @@ CodeGenerator::CodeGenerator(Frame* frame, Linkage* linkage,
       last_lazy_deopt_pc_(0),
       jump_tables_(nullptr),
       ools_(nullptr),
-      osr_pc_offset_(-1) {
+      osr_pc_offset_(-1),
+      deconstruct_frame_when_leaving_(true),
+      deconstruct_frame_between_blocks_(false) {
   for (int i = 0; i < code->InstructionBlockCount(); ++i) {
     new (&labels_[i]) Label;
   }
-  if (code->ContainsCall()) {
-    frame->MarkNeedsFrame();
-  }
 }
 
+void CodeGenerator::SetupFrameAccessForBlock(const InstructionBlock* block) {
+  if (block->needs_frame()) {
+    frame_access_state()->SetFrameAccessToFP();
+    frame_access_state()->set_used_frame_slots(
+        frame()->GetTotalFrameSlotCount());
+  } else {
+    frame_access_state()->SetFrameAccessToSP();
+    frame_access_state()->set_used_frame_slots(kElidedFrameSlots);
+  }
+}
 
 Handle<Code> CodeGenerator::GenerateCode() {
   CompilationInfo* info = this->info();
@@ -80,7 +89,10 @@ Handle<Code> CodeGenerator::GenerateCode() {
   }
   // Architecture-specific, linkage-specific prologue.
   info->set_prologue_offset(masm()->pc_offset());
-  AssemblePrologue();
+  // ARM64?
+  // frame()->AlignFrame(16);
+  // SetupStackPointer();
+  //
   if (linkage()->GetIncomingDescriptor()->InitializeRootRegister()) {
     masm()->InitializeRootRegister();

[Mircea Trofin, 2016/03/16 20:20:46]

What's the root register? Seems like something that should happen together with
frame setup.

[danno, 2016/03/16 21:20:56]

Nah, this is unrelated to the frame and should stay. This is the register that
common constants are loaded from, and just needs to be set up once in the
prologue.

[Mircea Trofin, 2016/03/16 21:51:00]

it seems that Simulator::CheckPCSComplianceAndRun cares about this ordering,
though.
Probably best to move it right after AssemblePrologue, at least for now.

   }
@@ -103,7 +115,6 @@ Handle<Code> CodeGenerator::GenerateCode() {
       DefineDeoptimizationLiteral(inlined.inlined_code_object_root);
     }
   }
-
   // Assemble all non-deferred blocks, followed by deferred ones.
   for (int deferred = 0; deferred < 2; ++deferred) {
     for (const InstructionBlock* block : code()->instruction_blocks()) {
@@ -143,10 +154,30 @@ Handle<Code> CodeGenerator::GenerateCode() {
         SNPrintF(buffer, " --");
         masm()->RecordComment(buffer_start);
       }
+
+      SetupFrameAccessForBlock(block);
+
       masm()->bind(GetLabel(current_block_));
+      if (block->must_construct_frame()) {
+        DCHECK(frame_access_state()->access_frame_with_fp());
+        AssemblePrologue();
+      }
+      set_deconstruct_frame_between_blocks(false);
+      set_deconstruct_frame_when_leaving(false);
+
       for (int i = block->code_start(); i < block->code_end(); ++i) {
-        AssembleInstruction(code()->InstructionAt(i));
+        Instruction* instr = code()->InstructionAt(i);
+        bool leaves_function =

[danno, 2016/03/16 18:18:11]

This should not be calculated here.  When the blocks are built, or when the
frame elision analysis happens, must_decontruct_frame should be (re)set to false
for the block if the block ends in a deopt or return. That simplifies this logic
a lot, or at least puts the decision in a better place, and I think it makes the
decision much cleaner, you don't have to check every instruction, just the last
one. i.e. somewhere you need:

Instruction* instr = code()->InstructionAt(block->last_instruction_index())
bool leaves_function = instr-> .... as before
if (leaves_function) block->set_must_deconstruct_frame(false);

[Mircea Trofin, 2016/03/16 20:20:46]

That makes sense, indeed. This is also a remnant of the older CL.

+            instr->arch_opcode() == ArchOpcode::kArchRet ||
+            instr->arch_opcode() == ArchOpcode::kArchDeoptimize;
+        if (i == block->last_instruction_index()) {
+          set_deconstruct_frame_when_leaving(block->must_deconstruct_frame());
+          set_deconstruct_frame_between_blocks(!leaves_function &&
+                                               block->must_deconstruct_frame());
+        }
+        AssembleInstruction(instr);
       }
+      AssembleDeconstructFrameBetweenBlocks();
     }
   }
 
@@ -767,15 +798,17 @@ int CodeGenerator::TailCallFrameStackSlotDelta(int stack_param_delta) {
   // 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;
+      has_frame ? (frame_access_state()->used_frame_slots() - pc_slots) : 0;

[danno, 2016/03/16 21:20:56]

If you use frame_access_state->has_frame(), you can keep the logic as it was
before.

   // 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_) {
+    : frame_(gen->frame()),
+      frame_access_state_(gen->frame_access_state()),
+      masm_(gen->masm()),
+      next_(gen->ools_) {
   gen->ools_ = this;
 }