[turbofan] Frame elision for code stubs

src/compiler/frame.h

Index: src/compiler/frame.h
diff --git a/src/compiler/frame.h b/src/compiler/frame.h
index 77dccaa629cc0c36e2cb604dc5e3ebf3e7065055..774b7728875cd3f5c7a2394cae779cfcf90a6195 100644
--- a/src/compiler/frame.h
+++ b/src/compiler/frame.h
@@ -81,26 +81,13 @@ class Frame : public ZoneObject {
   explicit Frame(int fixed_frame_size_in_slots,
                  const CallDescriptor* descriptor);
 
-  inline bool needs_frame() const { return needs_frame_; }
-  inline void MarkNeedsFrame() { needs_frame_ = true; }
-
   inline int GetTotalFrameSlotCount() const { return frame_slot_count_; }
 
-  inline int GetSPToFPSlotCount() const {

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

I don't think you should remove this, see below.

-    return GetTotalFrameSlotCount() -
-           StandardFrameConstants::kFixedSlotCountAboveFp;
-  }
   inline int GetSavedCalleeRegisterSlotCount() const {
     return callee_saved_slot_count_;
   }
   inline int GetSpillSlotCount() const { return spill_slot_count_; }
 
-  inline void SetElidedFrameSizeInSlots(int slots) {
-    DCHECK_EQ(0, callee_saved_slot_count_);
-    DCHECK_EQ(0, spill_slot_count_);
-    frame_slot_count_ = slots;
-  }
-
   void SetAllocatedRegisters(BitVector* regs) {
     DCHECK(allocated_registers_ == nullptr);
     allocated_registers_ = regs;
@@ -120,21 +107,18 @@ class Frame : public ZoneObject {
     int alignment_slots = alignment / kPointerSize;
     int delta = alignment_slots - (frame_slot_count_ & (alignment_slots - 1));
     if (delta != alignment_slots) {
-      DCHECK(needs_frame_);
       frame_slot_count_ += delta;
     }
     return delta;
   }
 
   void AllocateSavedCalleeRegisterSlots(int count) {
-    needs_frame_ = true;
     frame_slot_count_ += count;
     callee_saved_slot_count_ += count;
   }
 
   int AllocateSpillSlot(int width) {
     DCHECK_EQ(0, callee_saved_slot_count_);
-    needs_frame_ = true;
     int frame_slot_count_before = frame_slot_count_;
     int slot = AllocateAlignedFrameSlot(width);
     spill_slot_count_ += (frame_slot_count_ - frame_slot_count_before);
@@ -146,7 +130,6 @@ class Frame : public ZoneObject {
   int ReserveSpillSlots(size_t slot_count) {
     DCHECK_EQ(0, callee_saved_slot_count_);
     DCHECK_EQ(0, spill_slot_count_);
-    needs_frame_ = true;
     spill_slot_count_ += static_cast<int>(slot_count);
     frame_slot_count_ += static_cast<int>(slot_count);
     return frame_slot_count_ - 1;
@@ -168,7 +151,6 @@ class Frame : public ZoneObject {
   }
 
  private:
-  bool needs_frame_;
   int frame_slot_count_;
   int callee_saved_slot_count_;
   int spill_slot_count_;
@@ -210,7 +192,10 @@ class FrameOffset {
 class FrameAccessState : public ZoneObject {
  public:
   explicit FrameAccessState(Frame* const frame)
-      : frame_(frame), access_frame_with_fp_(false), sp_delta_(0) {
+      : frame_(frame),
+        access_frame_with_fp_(false),
+        sp_delta_(0),
+        used_frame_slots_(0) {
     SetFrameAccessToDefault();
   }
 
@@ -221,12 +206,16 @@ class FrameAccessState : public ZoneObject {
   void IncreaseSPDelta(int amount) { sp_delta_ += amount; }
 
   bool access_frame_with_fp() const { return access_frame_with_fp_; }
+  int used_frame_slots() const { return used_frame_slots_; }
+  void set_used_frame_slots(int used_slots) { used_frame_slots_ = used_slots; }

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

I don't think you need to track used_frame_slots explicitly if you have
has_frame_ as described elsewhere in my comments, I would prefer to have the
logic leak out as it kind of has in your CL. 

+
   void SetFrameAccessToDefault();
   void SetFrameAccessToFP() { access_frame_with_fp_ = true; }
   void SetFrameAccessToSP() { access_frame_with_fp_ = false; }
 
   int GetSPToFPSlotCount() const {
-    return frame_->GetSPToFPSlotCount() + sp_delta();
+    return used_frame_slots() - StandardFrameConstants::kFixedSlotCountAboveFp +

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

I don't understand this change. The frame size frame_->GetSPToFPSlotCount()
should be constant and set up knowing the the maximum slot count for the frame
when it's present, regardless of whether there is a frame in this very moment or
not. 

So the implementation should make the decision something like this:

DCHECK(access_frame_with_fp());
int frame_slot_count =  has_frame_ ? frame_->GetSPToFPSlotCount() :
kElidedFrameSlotCount;
return frame_slot_count + sp_delta();

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

Used_frame_slots is a crutch I needed to make sure I understood how things
worked. 

Having said that, let's talk  about Frame::GetSPToFPSlotCount. With frame
elision, the notion seems confusing to me, when exposed on Frame: what "FP" to
diff slots from SP? Turns out, there's only one consumer of this API, which is
here, on FrameAccessState. So I'd argue we remove the one on Frame, and have
just this one here, with the implementation you suggested.

Thoughts?

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

Yeah, I can see that now. That sounds OK. 

+           sp_delta();
   }
   int GetSPToFPOffset() const { return GetSPToFPSlotCount() * kPointerSize; }
 
@@ -240,6 +229,7 @@ class FrameAccessState : public ZoneObject {
   Frame* const frame_;
   bool access_frame_with_fp_;
   int sp_delta_;
+  int used_frame_slots_;
 };
 }  // namespace compiler
 }  // namespace internal