[turbofan] CodeGenerator: Frame setup refactoring

src/compiler/frame.h

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef V8_COMPILER_FRAME_H_
 #define V8_COMPILER_FRAME_H_
 
 #include "src/bit-vector.h"
 #include "src/frames.h"
 
@@ skipping 117 matching lines @@
   int AlignFrame(int alignment = kDoubleSize);
 
   int ReserveSpillSlots(size_t slot_count) {
     DCHECK_EQ(0, callee_saved_slot_count_);
     DCHECK_EQ(0, spill_slot_count_);
     spill_slot_count_ += static_cast<int>(slot_count);
     frame_slot_count_ += static_cast<int>(slot_count);
     return frame_slot_count_ - 1;
   }
 
+  int stack_shrink_slots() const { return stack_shrink_slots_; }
+  void set_stack_shrink_slots(int value) { stack_shrink_slots_ = value; }
+
   static const int kContextSlot = 2 + StandardFrameConstants::kCPSlotCount;
   static const int kJSFunctionSlot = 3 + StandardFrameConstants::kCPSlotCount;
 
  private:
   int AllocateAlignedFrameSlot(int width) {
     DCHECK(width == 4 || width == 8);
     // Skip one slot if necessary.
     if (width > kPointerSize) {
       DCHECK(width == kPointerSize * 2);
       frame_slot_count_++;
       frame_slot_count_ |= 1;
     }
     return frame_slot_count_++;
   }
 
  private:
   int frame_slot_count_;
   int callee_saved_slot_count_;
   int spill_slot_count_;
+  int stack_shrink_slots_;

[danno, 2016/04/01 15:56:26]

I don't think this has to/should be separate from, say, spill_slot_count_. There
are three parts of the stack documented at the top of this class, the header,
spill slots and callee-saved. I think you want to keep just three areas, but
fold stack_shrink_slots_ actually can just get folded spill_slot_count, because
that's how OSR ends up handling these slots... just like spill slots (or it
should). Upshot: Please don't add anything to this class if you don't have to or
can model with something else that's already there.

In fact, I think as part of your cleanup, you probably want to get rid of
callee_saved_slot_count_, too, from here. You can model callee saved registers
better in the FrameAccessState. You can imagine a world where we only save
callee-saved register around the blocks that actually use them, and elide the
saves elsewhere, just like we do for frame elision. In FrameAccessState, you
just need to add a callee_saved_slot_count_ that gets added to 
callee_saved_slot_count_ at the right points. Then FrameAccessState, which is
mutable, can save/unsave callee-saved register whenever it wants.

[Mircea Trofin, 2016/04/01 18:27:39]

Do we even have a reason to handle callee saved registers as part of the frame
altogether? Here's an alternative: in the instruction sequence, parameters
passed by register appear as constraints after archNop instructions. 
[eax] = archNop
(v1 = eax)
...

We add constraints before the ret instruction to re-assign back to the incoming
register the virtual register value, and then let the register allocator handle
spilling. The concept of "callee saved registers" goes away from frames and code
generation:

()(eax = v1)
archRet

Thoughts?

[Mircea Trofin, 2016/04/16 04:25:10]

Done, except with the moving of the callee saved registers stuff to
FrameAccessState. I'd prefer doing that
when/if we add the logic to selectively spill them, rather than whole sale
spilling them, as we do today.

   BitVector* allocated_registers_;
   BitVector* allocated_double_registers_;
 
   DISALLOW_COPY_AND_ASSIGN(Frame);
 };
 
 
 // Represents an offset from either the stack pointer or frame pointer.
 class FrameOffset {
  public:
@@ skipping 17 matching lines @@
   int offset_;  // Encodes SP or FP in the low order bit.
 
   static const int kFromSp = 1;
   static const int kFromFp = 0;
 };
 
 // Encapsulates the mutable state maintained during code generation about the
 // current function's frame.
 class FrameAccessState : public ZoneObject {
  public:
-  explicit FrameAccessState(Frame* const frame)
+  explicit FrameAccessState(const Frame* const frame)
       : frame_(frame),
         access_frame_with_fp_(false),
         sp_delta_(0),
         has_frame_(false) {}
 
-  Frame* frame() const { return frame_; }
+  const Frame* frame() const { return frame_; }
   void MarkHasFrame(bool state);
 
   int sp_delta() const { return sp_delta_; }
   void ClearSPDelta() { sp_delta_ = 0; }
   void IncreaseSPDelta(int amount) { sp_delta_ += amount; }
 
   bool access_frame_with_fp() const { return access_frame_with_fp_; }
 
   // Regardless of how we access slots on the stack - using sp or fp - do we
   // have a frame, at the current stage in code generation.
@@ skipping 11 matching lines @@
   }
   int GetSPToFPOffset() const { return GetSPToFPSlotCount() * kPointerSize; }
 
   // Get the frame offset for a given spill slot. The location depends on the
   // calling convention and the specific frame layout, and may thus be
   // architecture-specific. Negative spill slots indicate arguments on the
   // caller's frame.
   FrameOffset GetFrameOffset(int spill_slot) const;
 
  private:
-  Frame* const frame_;
+  const Frame* const frame_;
   bool access_frame_with_fp_;
   int sp_delta_;
   bool has_frame_;
 };
 }  // namespace compiler
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_COMPILER_FRAME_H_