Generators save and restore stack handlers

Generators save and restore stack handlers

This CL adds machinery to unwind stack handlers from the stack and store
them into a generator's operand array.  It also includes routines to
reinstate them.  Together this allows generators to yield within
try/catch and try/finally blocks.

BUG=v8:2355
R=mstarzinger@chromium.org
TEST=mjsunit/harmony/generators-iteration

Committed: 14586

[wingo, 2013-04-26 14:12:48]

The last major piece of basic generators functionality, I think.  Enjoy!

[wingo, 2013-04-29 14:09:00]

Updated patch doesn't skip the write barrier when saving the operand stack.

[wingo, 2013-05-02 15:53:06]

Updated patch rebased on top of https://codereview.chromium.org/13870007/.

[Michael Starzinger, 2013-05-07 09:55:44]

https://codereview.chromium.org/14031028/diff/6001/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/14031028/diff/6001/src/frames.cc#newcode852
src/frames.cc:852: const int kStackHandlerSlotCount =
Move the "kStackHandlerSlotCount" constant into the StackHandlerConstants class
(e.g. StackHandlerConstants::kSlotCount).

https://codereview.chromium.org/14031028/diff/6001/src/frames.cc#newcode883
src/frames.cc:883: const int kStackHandlerSlotCount =
Likewise.

https://codereview.chromium.org/14031028/diff/6001/src/frames.cc#newcode1513
src/frames.cc:1513: void StackHandler::Unwind(Isolate* isolate, FixedArray*
array, int offset,
style: Better put each argument on a single line, makes it easier to read (see
e.g. StackFrame::IteratePc).

https://codereview.chromium.org/14031028/diff/6001/src/frames.cc#newcode1534
src/frames.cc:1534: Object* StackHandler::Rewind(Isolate* isolate, FixedArray*
array, int offset,
style: Likewise.

https://codereview.chromium.org/14031028/diff/6001/src/frames.cc#newcode1545
src/frames.cc:1545: static_cast<unsigned>(int_state << kSmiTagSize) >>
kSmiTagSize;
I think this conversion is broken on 64-bit systems. In general this conversion
(and the one above) scare me a bit. Can we rewrite this using just static casts?

https://codereview.chromium.org/14031028/diff/6001/src/frames.cc#newcode1550
src/frames.cc:1550: *isolate->handler_address();
nit: Indentation is off.

https://codereview.chromium.org/14031028/diff/6001/src/frames.h
File src/frames.h (right):

https://codereview.chromium.org/14031028/diff/6001/src/frames.h#newcode135
src/frames.h:135: // handlers.
nit: How about shortening the comment to just "Generator support to preserve
stack handlers."

https://codereview.chromium.org/14031028/diff/6001/src/frames.h#newcode137
src/frames.h:137: Object *data) const;
Can we make "data" to be of type Smi? That would better describe it's usage.

https://codereview.chromium.org/14031028/diff/6001/src/frames.h#newcode139
src/frames.h:139: Address fp);
nit: Should fit into one line.

https://codereview.chromium.org/14031028/diff/6001/src/frames.h#newcode551
src/frames.h:551: void SaveOperandStack(FixedArray* store, int*
stack_handler_index) const;
nit: Maybe add a one-liner comment "Generator support to preserve operand
stack."

[wingo, 2013-05-07 12:40:31]

Updated patch addresses comments.  By re-using the slot that holds the FP when a
stack handler is active, we avoid the dodgy casts and don't have to change the
state encoding.

[wingo, 2013-05-07 13:09:27]

Following your suggestion to change to Smi* instead of Object*, it seems clearer
if Unwind / Rewind just use int.  Updated patch does that.

[Michael Starzinger, 2013-05-07 20:21:18]

LGTM if final nits are addressed.

The new way of storing the state split into "kind" and "index" is much easier to
read and gives us back the full width for index, I like that much more.

https://codereview.chromium.org/14031028/diff/16001/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/14031028/diff/16001/src/frames.cc#newcode1521
src/frames.cc:1521: array->set(offset + 2,
Smi::FromInt(static_cast<int>(index())));  // state
nit: The comment at the end of the line is outdated.

https://codereview.chromium.org/14031028/diff/16001/src/frames.cc#newcode1523
src/frames.cc:1523: array->set(offset + 4,
Smi::FromInt(static_cast<int>(kind())));   // fp
nit: The comment at the end of the line is outdated.

https://codereview.chromium.org/14031028/diff/16001/src/frames.cc#newcode1536
src/frames.cc:1536: Smi *prev_handler_offset = Smi::cast(array->get(offset));
style: We stick the '*' onto the type (i.e. to the left).

https://codereview.chromium.org/14031028/diff/16001/src/frames.cc#newcode1543
src/frames.cc:1543:
IndexField::encode(static_cast<unsigned>(smi_index->value()));
nit: Indentation is off.

[Michael Starzinger, 2013-05-07 20:25:23]

LGTM.

https://codereview.chromium.org/14031028/diff/16001/src/frames.cc
File src/frames.cc (right):

https://codereview.chromium.org/14031028/diff/16001/src/frames.cc#newcode1521
src/frames.cc:1521: array->set(offset + 2,
Smi::FromInt(static_cast<int>(index())));  // state
On 2013/05/07 20:21:18, Michael Starzinger wrote:
> nit: The comment at the end of the line is outdated.

Ah, OK, now I got it, the comments reflect the original usage of the slot inside
the stack handler. Makes sense.

Please ignore this comment.

https://codereview.chromium.org/14031028/diff/16001/src/frames.cc#newcode1523
src/frames.cc:1523: array->set(offset + 4,
Smi::FromInt(static_cast<int>(kind())));   // fp
On 2013/05/07 20:21:18, Michael Starzinger wrote:
> nit: The comment at the end of the line is outdated.

Please ignore this comment.

[wingo, 2013-05-08 08:08:34]

Committed patchset #7 manually as r14586 (presubmit successful).