[runtime] Unify and simplify how frames are marked

[runtime] Unify and simplify how frames are marked

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach
tends to make stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:

* Ensuring that there are only two fundamentally different
  types of frames, a "standard" frame and a "typed" frame.
  Standard frames, as before, contain both a context and
  function pointer. Typed frames contain only a minimum
  of a smi marker in the position immediately below the fp
  where the context is in standard frames.
* Only interpreted, full codegen, and optimized Crankshaft and
  TurboFan JavaScript frames use the "standard" format. All
  other frames use the type frame format with an explicit
  marker.
* Typed frames can contain one or more values below the
  type marker. There is new magic macro machinery in
  frames.h that simplifies defining the offsets of these fields
  in typed frames.
* A new flag in the CallDescriptor enables specifying whether
  a frame is a standard frame or a typed frame. Secondary
  register location spilling is now only enabled for standard
  frames.
* A zillion places in the code have been updated to deal with
  the fact that most code stubs and internal frames use the
  typed frame format. This includes changes in the
  deoptimizer, debugger, and liveedit.
* StandardFrameConstants::kMarkerOffset is deprecated,
  (CommonFrameConstants::kContextOrFrameTypeOffset
  and StandardFrameConstants::kFrameOffset are now used
  in its stead).

LOG=N

Committed: https://crrev.com/9dcd0857d64a117801bf91cb6caf7d6578809cb1
Cr-Commit-Position: refs/heads/master@{#34571}

[danno, 2016-02-22 10:28:14]

Description was changed from

==========
Uniform frame marker
==========

to

==========
[runtime] Unify and simplify how frames are marked.

Before this CL, various code stubs used different techniques for marking their
frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach tends to make
stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:
==========

[danno, 2016-02-22 14:00:30]

Description was changed from

==========
[runtime] Unify and simplify how frames are marked.

Before this CL, various code stubs used different techniques for marking their
frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach tends to make
stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:
==========

to

==========
[runtime] Unify and simplify how frames are marked.

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach tends to make
stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:
==========

[danno, 2016-02-22 14:15:53]

Description was changed from

==========
[runtime] Unify and simplify how frames are marked.

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach tends to make
stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:
==========

to

==========
[runtime] Unify and simplify how frames are marked.

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach
tends to make stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:

* Ensuring that there are only two fundamentally different
  types of frames, a "standard" frame and a "typed" frame.
  Standard frames, as before, contain both a context and
  function pointer. Typed frames contain only a minimum
  of a smi marker in the position immediately below the fp
  where the context is in standard frames.
* Only interpreted, full codegen, and optimized Crankshaft and
  TurboFan JavaScript frames use the "standard" format. All
  other frames use explicit the type frame format with and
  explicit marker.
* Typed frames can contain one or move values below the
  type marker. There is new magic macro machinery that 
  simplifies defining the offsets of these fields in typed frames.
* A new flag in the CallDescriptor enables specifying whether
  a frame is a standard frame or a typed frame. Secondary
  register location spilling is now only enabled for standard
  frames.
* A zillion places in the code have been updated to deal with
  the fact that most code stubs and internal frames use the
  typed frame format. This includes changes in the
  deoptimizer, debugger, liveedit.
* StandardFrameConstants::kMarkerOffset is deprecated,
  (CommonFrameConstants::kContextOrFrameTypeOffset
  and StandardFrameConstants::kFrameOffset are now used
  in its stead).

LOG=N
==========

[danno, 2016-02-22 14:15:53]

danno@chromium.org changed reviewers:
+ bmeurer@chromium.org, mstarzinger@chromium.org, verwaest@chromium.org

[danno, 2016-02-22 14:17:16]

Description was changed from

==========
[runtime] Unify and simplify how frames are marked.

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach
tends to make stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:

* Ensuring that there are only two fundamentally different
  types of frames, a "standard" frame and a "typed" frame.
  Standard frames, as before, contain both a context and
  function pointer. Typed frames contain only a minimum
  of a smi marker in the position immediately below the fp
  where the context is in standard frames.
* Only interpreted, full codegen, and optimized Crankshaft and
  TurboFan JavaScript frames use the "standard" format. All
  other frames use explicit the type frame format with and
  explicit marker.
* Typed frames can contain one or move values below the
  type marker. There is new magic macro machinery that 
  simplifies defining the offsets of these fields in typed frames.
* A new flag in the CallDescriptor enables specifying whether
  a frame is a standard frame or a typed frame. Secondary
  register location spilling is now only enabled for standard
  frames.
* A zillion places in the code have been updated to deal with
  the fact that most code stubs and internal frames use the
  typed frame format. This includes changes in the
  deoptimizer, debugger, liveedit.
* StandardFrameConstants::kMarkerOffset is deprecated,
  (CommonFrameConstants::kContextOrFrameTypeOffset
  and StandardFrameConstants::kFrameOffset are now used
  in its stead).

LOG=N
==========

to

==========
[runtime] Unify and simplify how frames are marked.

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach
tends to make stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:

* Ensuring that there are only two fundamentally different
  types of frames, a "standard" frame and a "typed" frame.
  Standard frames, as before, contain both a context and
  function pointer. Typed frames contain only a minimum
  of a smi marker in the position immediately below the fp
  where the context is in standard frames.
* Only interpreted, full codegen, and optimized Crankshaft and
  TurboFan JavaScript frames use the "standard" format. All
  other frames use the type frame format with an explicit
  marker.
* Typed frames can contain one or move values below the
  type marker. There is new magic macro machinery in
  frames.h that simplifies defining the offsets of these fields
  in typed frames.
* A new flag in the CallDescriptor enables specifying whether
  a frame is a standard frame or a typed frame. Secondary
  register location spilling is now only enabled for standard
  frames.
* A zillion places in the code have been updated to deal with
  the fact that most code stubs and internal frames use the
  typed frame format. This includes changes in the
  deoptimizer, debugger, and liveedit.
* StandardFrameConstants::kMarkerOffset is deprecated,
  (CommonFrameConstants::kContextOrFrameTypeOffset
  and StandardFrameConstants::kFrameOffset are now used
  in its stead).

LOG=N
==========

[danno, 2016-02-22 14:30:07]

PTAL, right now only the ia32 port is complete. I'd like to get feedback before
doing the rest of the platform ports.

Start in frames.h if you'd like to get an overview of what's changed.

[danno, 2016-02-22 14:30:35]

Description was changed from

==========
[runtime] Unify and simplify how frames are marked.

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach
tends to make stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:

* Ensuring that there are only two fundamentally different
  types of frames, a "standard" frame and a "typed" frame.
  Standard frames, as before, contain both a context and
  function pointer. Typed frames contain only a minimum
  of a smi marker in the position immediately below the fp
  where the context is in standard frames.
* Only interpreted, full codegen, and optimized Crankshaft and
  TurboFan JavaScript frames use the "standard" format. All
  other frames use the type frame format with an explicit
  marker.
* Typed frames can contain one or move values below the
  type marker. There is new magic macro machinery in
  frames.h that simplifies defining the offsets of these fields
  in typed frames.
* A new flag in the CallDescriptor enables specifying whether
  a frame is a standard frame or a typed frame. Secondary
  register location spilling is now only enabled for standard
  frames.
* A zillion places in the code have been updated to deal with
  the fact that most code stubs and internal frames use the
  typed frame format. This includes changes in the
  deoptimizer, debugger, and liveedit.
* StandardFrameConstants::kMarkerOffset is deprecated,
  (CommonFrameConstants::kContextOrFrameTypeOffset
  and StandardFrameConstants::kFrameOffset are now used
  in its stead).

LOG=N
==========

to

==========
[runtime] Unify and simplify how frames are marked

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach
tends to make stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:

* Ensuring that there are only two fundamentally different
  types of frames, a "standard" frame and a "typed" frame.
  Standard frames, as before, contain both a context and
  function pointer. Typed frames contain only a minimum
  of a smi marker in the position immediately below the fp
  where the context is in standard frames.
* Only interpreted, full codegen, and optimized Crankshaft and
  TurboFan JavaScript frames use the "standard" format. All
  other frames use the type frame format with an explicit
  marker.
* Typed frames can contain one or move values below the
  type marker. There is new magic macro machinery in
  frames.h that simplifies defining the offsets of these fields
  in typed frames.
* A new flag in the CallDescriptor enables specifying whether
  a frame is a standard frame or a typed frame. Secondary
  register location spilling is now only enabled for standard
  frames.
* A zillion places in the code have been updated to deal with
  the fact that most code stubs and internal frames use the
  typed frame format. This includes changes in the
  deoptimizer, debugger, and liveedit.
* StandardFrameConstants::kMarkerOffset is deprecated,
  (CommonFrameConstants::kContextOrFrameTypeOffset
  and StandardFrameConstants::kFrameOffset are now used
  in its stead).

LOG=N
==========

[Benedikt Meurer, 2016-02-22 17:32:24]

Description was changed from

==========
[runtime] Unify and simplify how frames are marked

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach
tends to make stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:

* Ensuring that there are only two fundamentally different
  types of frames, a "standard" frame and a "typed" frame.
  Standard frames, as before, contain both a context and
  function pointer. Typed frames contain only a minimum
  of a smi marker in the position immediately below the fp
  where the context is in standard frames.
* Only interpreted, full codegen, and optimized Crankshaft and
  TurboFan JavaScript frames use the "standard" format. All
  other frames use the type frame format with an explicit
  marker.
* Typed frames can contain one or move values below the
  type marker. There is new magic macro machinery in
  frames.h that simplifies defining the offsets of these fields
  in typed frames.
* A new flag in the CallDescriptor enables specifying whether
  a frame is a standard frame or a typed frame. Secondary
  register location spilling is now only enabled for standard
  frames.
* A zillion places in the code have been updated to deal with
  the fact that most code stubs and internal frames use the
  typed frame format. This includes changes in the
  deoptimizer, debugger, and liveedit.
* StandardFrameConstants::kMarkerOffset is deprecated,
  (CommonFrameConstants::kContextOrFrameTypeOffset
  and StandardFrameConstants::kFrameOffset are now used
  in its stead).

LOG=N
==========

to

==========
[runtime] Unify and simplify how frames are marked

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach
tends to make stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:

* Ensuring that there are only two fundamentally different
  types of frames, a "standard" frame and a "typed" frame.
  Standard frames, as before, contain both a context and
  function pointer. Typed frames contain only a minimum
  of a smi marker in the position immediately below the fp
  where the context is in standard frames.
* Only interpreted, full codegen, and optimized Crankshaft and
  TurboFan JavaScript frames use the "standard" format. All
  other frames use the type frame format with an explicit
  marker.
* Typed frames can contain one or more values below the
  type marker. There is new magic macro machinery in
  frames.h that simplifies defining the offsets of these fields
  in typed frames.
* A new flag in the CallDescriptor enables specifying whether
  a frame is a standard frame or a typed frame. Secondary
  register location spilling is now only enabled for standard
  frames.
* A zillion places in the code have been updated to deal with
  the fact that most code stubs and internal frames use the
  typed frame format. This includes changes in the
  deoptimizer, debugger, and liveedit.
* StandardFrameConstants::kMarkerOffset is deprecated,
  (CommonFrameConstants::kContextOrFrameTypeOffset
  and StandardFrameConstants::kFrameOffset are now used
  in its stead).

LOG=N
==========

[Benedikt Meurer, 2016-02-22 18:16:39]

LGTM, I like it. Nice that you do these refactorings now!

https://codereview.chromium.org/1696043002/diff/440001/src/compiler/linkage.cc
File src/compiler/linkage.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/compiler/linkage.c...
src/compiler/linkage.cc:436: if ((incoming_->flags() &
CallDescriptor::kHasStandardFrameHeader) == 0) {
Nit: For "flags" I'd prefer to not use explicit comparison against 0 to
distinguish them easily from integer bitmasking.

https://codereview.chromium.org/1696043002/diff/440001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/compiler/pipeline....
src/compiler/pipeline.cc:283: fixed_frame_size = (((descriptor->flags() &
Nit: For "flags" I'd prefer to not use explicit comparison against 0 to
distinguish them easily from integer bitmasking.

https://codereview.chromium.org/1696043002/diff/440001/src/crankshaft/ia32/li...
File src/crankshaft/ia32/lithium-codegen-ia32.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/crankshaft/ia32/li...
src/crankshaft/ia32/lithium-codegen-ia32.cc:165: __ push(ebp);  // Caller's
frame pointer.
Awesome!

[Michael Starzinger, 2016-02-23 10:57:33]

LGTM. I like it. Just nits.

https://codereview.chromium.org/1696043002/diff/440001/src/compiler/ia32/code...
File src/compiler/ia32/code-generator-ia32.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/compiler/ia32/code...
src/compiler/ia32/code-generator-ia32.cc:1469: // Assemble a prologue similar
the to cdecl calling convention.
nit: Comment is outdated, it follows our internal calling convection.

https://codereview.chromium.org/1696043002/diff/440001/src/debug/liveedit.cc
File src/debug/liveedit.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/debug/liveedit.cc#...
src/debug/liveedit.cc:1565: //  int new_frame_size =
LiveEdit::kFrameDropperFrameSize * kPointerSize;
nit: Looks like left-over code. Can we drop this line?

https://codereview.chromium.org/1696043002/diff/440001/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/frames.cc#newcode655
src/frames.cc:655: // Visit the outgoing parameters.
nit: Comment is outdated.

https://codereview.chromium.org/1696043002/diff/440001/src/frames.h
File src/frames.h (right):

https://codereview.chromium.org/1696043002/diff/440001/src/frames.h#newcode154
src/frames.h:154: static const int kCallerPCOffset = +1 * kFPOnStackSize;
nit: "kCallerPCOffset = kCallerFPOffset + 1 * kFPOnStackSize", just for
consistency.

https://codereview.chromium.org/1696043002/diff/440001/src/frames.h#newcode302
src/frames.h:302: //  static const int kFunctionOffset =
TYPED_FRAME_PUSHED_VALUE_OFFSET(3);
nit: Looks like leftover, can we drop this line and the empty newline above?

https://codereview.chromium.org/1696043002/diff/440001/src/ia32/macro-assembl...
File src/ia32/macro-assembler-ia32.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/ia32/macro-assembl...
src/ia32/macro-assembler-ia32.cc:1067: DCHECK(ExitFrameConstants::kCodeOffset ==
-3 * kPointerSize);
nit: DCHECK_EQ

[balazs.kilvady, 2016-02-26 19:35:49]

Please find the MIPS port at https://codereview.chromium.org/1736323002/
I run .quickcheck tests on mipsel and mips64el simulators and all test passed.
I used Push(, , )/Pop(, , ) instead of MultiPush()/MultiPop() in
MacroAssembler::[Push|Pop]*Frame() functions and I think that code could be used
for arm, arm64 as well.

[balazs.kilvady, 2016-02-26 19:35:52]

Please find the MIPS port at https://codereview.chromium.org/1736323002/
I run .quickcheck tests on mipsel and mips64el simulators and all test passed.
I used Push(, , )/Pop(, , ) instead of MultiPush()/MultiPop() in
MacroAssembler::[Push|Pop]*Frame() functions and I think that code could be used
for arm, arm64 as well.

[danno, 2016-03-07 09:33:38]

Feedback addressed, some tweaks needed to pass all test and to merge with
latest. Please take another look.

https://codereview.chromium.org/1696043002/diff/440001/src/compiler/ia32/code...
File src/compiler/ia32/code-generator-ia32.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/compiler/ia32/code...
src/compiler/ia32/code-generator-ia32.cc:1469: // Assemble a prologue similar
the to cdecl calling convention.
On 2016/02/23 10:57:33, Michael Starzinger wrote:
> nit: Comment is outdated, it follows our internal calling convection.

Done.

https://codereview.chromium.org/1696043002/diff/440001/src/compiler/linkage.cc
File src/compiler/linkage.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/compiler/linkage.c...
src/compiler/linkage.cc:436: if ((incoming_->flags() &
CallDescriptor::kHasStandardFrameHeader) == 0) {
On 2016/02/22 18:16:38, Benedikt Meurer wrote:
> Nit: For "flags" I'd prefer to not use explicit comparison against 0 to
> distinguish them easily from integer bitmasking.

Done.

https://codereview.chromium.org/1696043002/diff/440001/src/compiler/pipeline.cc
File src/compiler/pipeline.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/compiler/pipeline....
src/compiler/pipeline.cc:283: fixed_frame_size = (((descriptor->flags() &
On 2016/02/22 18:16:38, Benedikt Meurer wrote:
> Nit: For "flags" I'd prefer to not use explicit comparison against 0 to
> distinguish them easily from integer bitmasking.

Done.

https://codereview.chromium.org/1696043002/diff/440001/src/crankshaft/ia32/li...
File src/crankshaft/ia32/lithium-codegen-ia32.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/crankshaft/ia32/li...
src/crankshaft/ia32/lithium-codegen-ia32.cc:165: __ push(ebp);  // Caller's
frame pointer.
On 2016/02/22 18:16:39, Benedikt Meurer wrote:
> Awesome!

Acknowledged.

https://codereview.chromium.org/1696043002/diff/440001/src/debug/liveedit.cc
File src/debug/liveedit.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/debug/liveedit.cc#...
src/debug/liveedit.cc:1565: //  int new_frame_size =
LiveEdit::kFrameDropperFrameSize * kPointerSize;
On 2016/02/23 10:57:33, Michael Starzinger wrote:
> nit: Looks like left-over code. Can we drop this line?

Done.

https://codereview.chromium.org/1696043002/diff/440001/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/frames.cc#newcode655
src/frames.cc:655: // Visit the outgoing parameters.
On 2016/02/23 10:57:33, Michael Starzinger wrote:
> nit: Comment is outdated.

Done.

https://codereview.chromium.org/1696043002/diff/440001/src/frames.h
File src/frames.h (right):

https://codereview.chromium.org/1696043002/diff/440001/src/frames.h#newcode154
src/frames.h:154: static const int kCallerPCOffset = +1 * kFPOnStackSize;
On 2016/02/23 10:57:33, Michael Starzinger wrote:
> nit: "kCallerPCOffset = kCallerFPOffset + 1 * kFPOnStackSize", just for
> consistency.

Done.

https://codereview.chromium.org/1696043002/diff/440001/src/frames.h#newcode302
src/frames.h:302: //  static const int kFunctionOffset =
TYPED_FRAME_PUSHED_VALUE_OFFSET(3);
On 2016/02/23 10:57:33, Michael Starzinger wrote:
> nit: Looks like leftover, can we drop this line and the empty newline above?

Done.

https://codereview.chromium.org/1696043002/diff/440001/src/ia32/macro-assembl...
File src/ia32/macro-assembler-ia32.cc (right):

https://codereview.chromium.org/1696043002/diff/440001/src/ia32/macro-assembl...
src/ia32/macro-assembler-ia32.cc:1067: DCHECK(ExitFrameConstants::kCodeOffset ==
-3 * kPointerSize);
On 2016/02/23 10:57:33, Michael Starzinger wrote:
> nit: DCHECK_EQ

Done.

[danno, 2016-03-07 09:45:40]

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

[danno, 2016-03-07 09:45:42]

Adding Ben...

[titzer, 2016-03-07 10:48:43]

I really like this cleanup. The only wrinkle is the extra marker for WASM code
as discussed offline.

https://codereview.chromium.org/1696043002/diff/790001/src/arm/macro-assemble...
File src/arm/macro-assembler-arm.cc (right):

https://codereview.chromium.org/1696043002/diff/790001/src/arm/macro-assemble...
src/arm/macro-assembler-arm.cc:1619: // my require crawling past STUB frames.
s/my/may

https://codereview.chromium.org/1696043002/diff/790001/src/compiler/arm/code-...
File src/compiler/arm/code-generator-arm.cc (right):

https://codereview.chromium.org/1696043002/diff/790001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:1173: __
StubPrologue(info()->GetOutputStackFrameType());
Would it be possible to unify the two cases and put the logic of which prologue
sequence to generate completely into the macro assembler?

https://codereview.chromium.org/1696043002/diff/790001/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/1696043002/diff/790001/src/frames.cc#newcode434
src/frames.cc:434: case WASM_TO_JS:
As discussed offline, if these two frame types don't have marker cookies, we'll
have to do the code object lookup below beforehand.

[danno, 2016-03-08 07:04:45]

Please take another look. I did some more cleanup and removed the cookie from
WASM frames as discussed. There is certainly more that can be done, but let's
save that for another CL, this one is already a monster.

https://codereview.chromium.org/1696043002/diff/790001/src/arm/macro-assemble...
File src/arm/macro-assembler-arm.cc (right):

https://codereview.chromium.org/1696043002/diff/790001/src/arm/macro-assemble...
src/arm/macro-assembler-arm.cc:1619: // my require crawling past STUB frames.
On 2016/03/07 10:48:43, titzer wrote:
> s/my/may

Done.

https://codereview.chromium.org/1696043002/diff/790001/src/compiler/arm/code-...
File src/compiler/arm/code-generator-arm.cc (right):

https://codereview.chromium.org/1696043002/diff/790001/src/compiler/arm/code-...
src/compiler/arm/code-generator-arm.cc:1173: __
StubPrologue(info()->GetOutputStackFrameType());
On 2016/03/07 10:48:43, titzer wrote:
> Would it be possible to unify the two cases and put the logic of which
prologue
> sequence to generate completely into the macro assembler?

Yes, but I'd prefer to save that for a separate CL.

https://codereview.chromium.org/1696043002/diff/790001/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/1696043002/diff/790001/src/frames.cc#newcode434
src/frames.cc:434: case WASM_TO_JS:
On 2016/03/07 10:48:43, titzer wrote:
> As discussed offline, if these two frame types don't have marker cookies,
we'll
> have to do the code object lookup below beforehand.

Done.

[titzer, 2016-03-08 07:26:24]

lgtm

[danno, 2016-03-08 08:31:44]

The CQ bit was checked by danno@chromium.org

[danno, 2016-03-08 08:31:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1696043002/#ps870001
(title: "Fix merge problems")

[commit-bot: I haz the power, 2016-03-08 08:31:57]

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

[commit-bot: I haz the power, 2016-03-08 08:35:49]

Description was changed from

==========
[runtime] Unify and simplify how frames are marked

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach
tends to make stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:

* Ensuring that there are only two fundamentally different
  types of frames, a "standard" frame and a "typed" frame.
  Standard frames, as before, contain both a context and
  function pointer. Typed frames contain only a minimum
  of a smi marker in the position immediately below the fp
  where the context is in standard frames.
* Only interpreted, full codegen, and optimized Crankshaft and
  TurboFan JavaScript frames use the "standard" format. All
  other frames use the type frame format with an explicit
  marker.
* Typed frames can contain one or more values below the
  type marker. There is new magic macro machinery in
  frames.h that simplifies defining the offsets of these fields
  in typed frames.
* A new flag in the CallDescriptor enables specifying whether
  a frame is a standard frame or a typed frame. Secondary
  register location spilling is now only enabled for standard
  frames.
* A zillion places in the code have been updated to deal with
  the fact that most code stubs and internal frames use the
  typed frame format. This includes changes in the
  deoptimizer, debugger, and liveedit.
* StandardFrameConstants::kMarkerOffset is deprecated,
  (CommonFrameConstants::kContextOrFrameTypeOffset
  and StandardFrameConstants::kFrameOffset are now used
  in its stead).

LOG=N
==========

to

==========
[runtime] Unify and simplify how frames are marked

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach
tends to make stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:

* Ensuring that there are only two fundamentally different
  types of frames, a "standard" frame and a "typed" frame.
  Standard frames, as before, contain both a context and
  function pointer. Typed frames contain only a minimum
  of a smi marker in the position immediately below the fp
  where the context is in standard frames.
* Only interpreted, full codegen, and optimized Crankshaft and
  TurboFan JavaScript frames use the "standard" format. All
  other frames use the type frame format with an explicit
  marker.
* Typed frames can contain one or more values below the
  type marker. There is new magic macro machinery in
  frames.h that simplifies defining the offsets of these fields
  in typed frames.
* A new flag in the CallDescriptor enables specifying whether
  a frame is a standard frame or a typed frame. Secondary
  register location spilling is now only enabled for standard
  frames.
* A zillion places in the code have been updated to deal with
  the fact that most code stubs and internal frames use the
  typed frame format. This includes changes in the
  deoptimizer, debugger, and liveedit.
* StandardFrameConstants::kMarkerOffset is deprecated,
  (CommonFrameConstants::kContextOrFrameTypeOffset
  and StandardFrameConstants::kFrameOffset are now used
  in its stead).

LOG=N
==========

[commit-bot: I haz the power, 2016-03-08 08:35:51]

Committed patchset #45 (id:870001)

[commit-bot: I haz the power, 2016-03-08 08:36:42]

Description was changed from

==========
[runtime] Unify and simplify how frames are marked

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach
tends to make stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:

* Ensuring that there are only two fundamentally different
  types of frames, a "standard" frame and a "typed" frame.
  Standard frames, as before, contain both a context and
  function pointer. Typed frames contain only a minimum
  of a smi marker in the position immediately below the fp
  where the context is in standard frames.
* Only interpreted, full codegen, and optimized Crankshaft and
  TurboFan JavaScript frames use the "standard" format. All
  other frames use the type frame format with an explicit
  marker.
* Typed frames can contain one or more values below the
  type marker. There is new magic macro machinery in
  frames.h that simplifies defining the offsets of these fields
  in typed frames.
* A new flag in the CallDescriptor enables specifying whether
  a frame is a standard frame or a typed frame. Secondary
  register location spilling is now only enabled for standard
  frames.
* A zillion places in the code have been updated to deal with
  the fact that most code stubs and internal frames use the
  typed frame format. This includes changes in the
  deoptimizer, debugger, and liveedit.
* StandardFrameConstants::kMarkerOffset is deprecated,
  (CommonFrameConstants::kContextOrFrameTypeOffset
  and StandardFrameConstants::kFrameOffset are now used
  in its stead).

LOG=N
==========

to

==========
[runtime] Unify and simplify how frames are marked

Before this CL, various code stubs used different techniques
for marking their frames to enable stack-crawling and other
access to data in the frame. All of them were based on a abuse
of the "standard" frame representation, e.g. storing the a
context pointer immediately below the frame's fp, and a
function pointer after that. Although functional, this approach
tends to make stubs and builtins do an awkward, unnecessary
dance to appear like standard frames, even if they have
nothing to do with JavaScript execution.

This CL attempts to improve this by:

* Ensuring that there are only two fundamentally different
  types of frames, a "standard" frame and a "typed" frame.
  Standard frames, as before, contain both a context and
  function pointer. Typed frames contain only a minimum
  of a smi marker in the position immediately below the fp
  where the context is in standard frames.
* Only interpreted, full codegen, and optimized Crankshaft and
  TurboFan JavaScript frames use the "standard" format. All
  other frames use the type frame format with an explicit
  marker.
* Typed frames can contain one or more values below the
  type marker. There is new magic macro machinery in
  frames.h that simplifies defining the offsets of these fields
  in typed frames.
* A new flag in the CallDescriptor enables specifying whether
  a frame is a standard frame or a typed frame. Secondary
  register location spilling is now only enabled for standard
  frames.
* A zillion places in the code have been updated to deal with
  the fact that most code stubs and internal frames use the
  typed frame format. This includes changes in the
  deoptimizer, debugger, and liveedit.
* StandardFrameConstants::kMarkerOffset is deprecated,
  (CommonFrameConstants::kContextOrFrameTypeOffset
  and StandardFrameConstants::kFrameOffset are now used
  in its stead).

LOG=N

Committed: https://crrev.com/9dcd0857d64a117801bf91cb6caf7d6578809cb1
Cr-Commit-Position: refs/heads/master@{#34571}
==========

[commit-bot: I haz the power, 2016-03-08 08:36:44]

Patchset 45 (id:??) landed as
https://crrev.com/9dcd0857d64a117801bf91cb6caf7d6578809cb1
Cr-Commit-Position: refs/heads/master@{#34571}

[Jakob Kummerow, 2016-03-09 14:19:44]

https://codereview.chromium.org/1696043002/diff/870001/src/x64/macro-assemble...
File src/x64/macro-assembler-x64.cc (right):

https://codereview.chromium.org/1696043002/diff/870001/src/x64/macro-assemble...
src/x64/macro-assembler-x64.cc:4591: MemOperand(scratch,
CommonFrameConstants::kContextOrFrameTypeOffset));
This is crashing in the wild, because |scratch| can still contain a Smi, just
one that's != StackFrame::STUB. Specifically, crash reports show 0xc, 0xd, 0xe,
which if I'm counting correctly indicate INTERNAL, CONSTRUCT, and
ARGUMENTS_ADAPTOR frames.
Other platforms do a JumpIfNotSmi in their equivalent of lines 4583-4586 instead
of an equality comparison against StackFrame::STUB. Maybe that's how it should
be done here too?