[turbofan] CodeGenerator: Frame setup refactoring

src/compiler/frame.h

Index: src/compiler/frame.h
diff --git a/src/compiler/frame.h b/src/compiler/frame.h
index d413d3e0336841dee93a753b12a3b431dad99994..d4c37d4bbfd89e0434353a66403f776bafd55636 100644
--- a/src/compiler/frame.h
+++ b/src/compiler/frame.h
@@ -135,6 +135,9 @@ class Frame : public ZoneObject {
     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;
 
@@ -154,6 +157,7 @@ class Frame : public ZoneObject {
   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_;
 
@@ -191,13 +195,13 @@ class FrameOffset {
 // 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_; }
@@ -229,7 +233,7 @@ class FrameAccessState : public ZoneObject {
   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_;