[turbofan] Unify referencing of stack slots

[turbofan] Unify referencing of stack slots

Previously, it was not possible to specify StackSlotOperands for all
slots in both the caller and callee stacks. Specifically, the region
of the callee's stack including the saved return address, frame
pointer, function pointer and context pointer could not be addressed
by the register allocator/gap resolver.

In preparation for better tail call support, which will use the gap
resolver to reconcile outgoing parameters, this change makes it
possible to address all slots on the stack, because slots in the
previously inaccessible dead zone may become parameter slots for
outgoing tail calls. All caller stack slots are accessible as they
were before, with slot -1 corresponding to the last stack
parameter. Stack slot indices >= 0 access the callee stack, with slot
0 corresponding to the callee's saved return address, 1 corresponding
to the saved frame pointer, 2 corresponding to the current function
context, 3 corresponding to the frame marker/JSFunction, and slots 4
and above corresponding to spill slots.

The following changes were specifically	needed:

* Frame	has been changed to explicitly manage three areas of the
  callee frame, the fixed header, the spill slot area, and the
  callee-saved register area.
* Conversions from stack slot indices to fp offsets all now go through
  a common bottleneck: OptimizedFrame::StackSlotOffsetRelativeToFp
* The generation of deoptimization translation tables has been changed
  to support the new stack slot indexing scheme. Crankshaft, which
  doesn't support the new slot numbering in its register allocator,
  must adapt the indexes when creating translation tables.
* Callee-saved parameters are now kept below spill slots, not above,
  to support saving only the optimal set of used registers, which is
  only known after register allocation is finished and spill slots
  have been allocated.

Committed: https://crrev.com/cbbaf9ea6abbc0417ee5765a4c58f1dda939ead0
Cr-Commit-Position: refs/heads/master@{#30224}

[titzer, 2015-08-13 18:12:53]

titzer@chromium.org changed reviewers:
+ titzer@chromium.org

[titzer, 2015-08-13 18:12:53]

Mostly looking good. ARM64 is going to be fun with the JSP/CSP distinction.

[danno, 2015-08-14 13:08:58]

danno@chromium.org changed reviewers:
+ bmeurer@chromium.org, jarin@chromium.org

[danno, 2015-08-14 13:36:27]

PTAL, I think it's ready to go

[Benedikt Meurer, 2015-08-16 07:04:36]

LGTM with nits.
Very nice, I love it.

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h
File src/compiler/frame.h (right):

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h#n...
src/compiler/frame.h:44: // allocator and gab resolver with a SpillSlotOperand
containing its
Nit: gap resolver

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h#n...
src/compiler/frame.h:63: //   2   |   JSFunction    |   |                    |
Nit: Context

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h#n...
src/compiler/frame.h:65: //   3   |     Marker      |   v                    |
Nit: JSFunction/Marker

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/x64/code-...
File src/compiler/x64/code-generator-x64.cc (right):

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/x64/code-...
src/compiler/x64/code-generator-x64.cc:1466: }
Nit: add // namespace after } to make clang-format happy. Or maybe move it to
globals.h?

[Jarin, 2015-08-16 09:28:59]

almost lgtm.

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h
File src/compiler/frame.h (right):

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h#n...
src/compiler/frame.h:125: ++spilled_callee_register_slot_count_ += count;
How about 'frame_slot_count_ += count + 1;'? I am not even sure what you do here
is well-defined.

[Benedikt Meurer, 2015-08-16 09:31:34]

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h
File src/compiler/frame.h (right):

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h#n...
src/compiler/frame.h:125: ++spilled_callee_register_slot_count_ += count;
Oh indeed. Missed this one. Should really be

frame_slot_count_ += count + 1;

[titzer, 2015-08-17 10:58:43]

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h
File src/compiler/frame.h (right):

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h#n...
src/compiler/frame.h:84: static const int kFixedSlotCount =
I think this is dead now?

[danno, 2015-08-18 13:23:11]

Feedback addressed. I'll land this when all of the try bots are green.

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h
File src/compiler/frame.h (right):

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h#n...
src/compiler/frame.h:44: // allocator and gab resolver with a SpillSlotOperand
containing its
On 2015/08/16 at 07:04:35, Benedikt Meurer wrote:
> Nit: gap resolver

Done

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h#n...
src/compiler/frame.h:63: //   2   |   JSFunction    |   |                    |
On 2015/08/16 at 07:04:35, Benedikt Meurer wrote:
> Nit: Context

Done

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h#n...
src/compiler/frame.h:65: //   3   |     Marker      |   v                    |
On 2015/08/16 at 07:04:35, Benedikt Meurer wrote:
> Nit: JSFunction/Marker

Done

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h#n...
src/compiler/frame.h:84: static const int kFixedSlotCount =
On 2015/08/17 at 10:58:42, titzer wrote:
> I think this is dead now?

No, it's not. It is used to initialize the Frame with the initial number of
slots.

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h#n...
src/compiler/frame.h:125: ++spilled_callee_register_slot_count_ += count;
On 2015/08/16 at 09:31:34, Benedikt Meurer wrote:
> Oh indeed. Missed this one. Should really be
> 
> frame_slot_count_ += count + 1;

Fixed.

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/x64/code-...
File src/compiler/x64/code-generator-x64.cc (right):

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/x64/code-...
src/compiler/x64/code-generator-x64.cc:1466: }
On 2015/08/16 at 07:04:35, Benedikt Meurer wrote:
> Nit: add // namespace after } to make clang-format happy. Or maybe move it to
globals.h?

Done

[titzer, 2015-08-18 13:30:04]

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h
File src/compiler/frame.h (right):

https://codereview.chromium.org/1261923007/diff/290027/src/compiler/frame.h#n...
src/compiler/frame.h:84: static const int kFixedSlotCount =
On 2015/08/18 13:23:10, danno wrote:
> On 2015/08/17 at 10:58:42, titzer wrote:
> > I think this is dead now?
> 
> No, it's not. It is used to initialize the Frame with the initial number of
> slots.

In that case recommend moving it to the usage sites.

[danno, 2015-08-18 14:46:53]

The CQ bit was checked by danno@chromium.org

[danno, 2015-08-18 14:46:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, jarin@chromium.org
Link to the patchset: https://codereview.chromium.org/1261923007/#ps330001
(title: "Review feedback")

[commit-bot: I haz the power, 2015-08-18 14:46:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1261923007/330001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1261923007/330001

[commit-bot: I haz the power, 2015-08-18 14:48:02]

Committed patchset #18 (id:330001)

[commit-bot: I haz the power, 2015-08-18 14:48:19]

Patchset 18 (id:??) landed as
https://crrev.com/cbbaf9ea6abbc0417ee5765a4c58f1dda939ead0
Cr-Commit-Position: refs/heads/master@{#30224}

[MTBrandyberry, 2015-08-26 17:35:13]

mbrandy@us.ibm.com changed reviewers:
+ mbrandy@us.ibm.com, mcilroy@chromium.org

[MTBrandyberry, 2015-08-26 17:35:14]

This new slot numbering scheme does not appear to support frames with embedded
constant pool pointers?

[MTBrandyberry, 2015-08-28 15:39:38]

On 2015/08/26 17:35:14, mtbrandyberry wrote:
> This new slot numbering scheme does not appear to support frames with embedded
> constant pool pointers?

Embedded constant pool issue fixed via
https://codereview.chromium.org/1317123003/