Switch full-codegen from StackHandlers to handler table.

Switch full-codegen from StackHandlers to handler table.

This switches full-codegen to no longer push and pop StackHandler
markers onto the operand stack, but relies on a range-based handler
table instead. We only use StackHandlers in JSEntryStubs to mark the
transition from C to JS code.

Note that this makes deoptimization and OSR from within any try-block
work out of the box, makes the non-exception paths faster and should
overall be neutral on the memory footprint (pros).

On the other hand it makes the exception paths slower and actually
throwing and exception more expensive (cons).

R=yangguo@chromium.org
TEST=cctest/test-run-jsexceptions/DeoptTry

Committed: https://crrev.com/38a719f965d0a83ddac04392d5b9c5abe214281c
Cr-Commit-Position: refs/heads/master@{#27440}

[Michael Starzinger, 2015-03-16 19:13:25]

Note that the platform ports are still missing and one Promises test (i.e.
try-throw-reject-in-constructor) is still failing, but I would like to get a
first round of comments if possible. Thanks a bunch!

[Michael Starzinger, 2015-03-16 19:16:26]

mstarzinger@chromium.org changed reviewers:
+ wingo@igalia.com

[Michael Starzinger, 2015-03-16 19:16:26]

Andy Wingo: FYI about Generators.

[Michael Starzinger, 2015-03-16 19:22:51]

mstarzinger@chromium.org changed required reviewers:
+ yangguo@chromium.org

[wingo, 2015-03-17 09:41:26]

Generators part LGTM.

https://codereview.chromium.org/1010883002/diff/40001/src/frames.cc
File src/frames.cc (left):

https://codereview.chromium.org/1010883002/diff/40001/src/frames.cc#oldcode873
src/frames.cc:873: for (; i >= 0; i--) {
o/~ did you ever know, that you're my hero o/~

https://codereview.chromium.org/1010883002/diff/40001/src/runtime/runtime-gen...
File src/runtime/runtime-generator.cc (right):

https://codereview.chromium.org/1010883002/diff/40001/src/runtime/runtime-gen...
src/runtime/runtime-generator.cc:70: }
Hoo, nasty stuff.  ACK though :)

[Yang, 2015-03-17 10:21:23]

Non-generator non-turbofan parts lgtm. Comments. And can we have test cases with
nested try-catch pretty please? :)

https://codereview.chromium.org/1010883002/diff/40001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/1010883002/diff/40001/src/objects-inl.h#newco...
src/objects-inl.h:767: // a handler table array.
Maybe we want to introduce a special fixed array map? Then again that would just
be for debugging and assertions.

https://codereview.chromium.org/1010883002/diff/40001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1010883002/diff/40001/src/objects.cc#newcode8540
src/objects.cc:8540: for (int i = 0; i < length(); i += kRangeEntrySize) {
The handler table should be sorted by start_offset (which we could also assert).
We could end the loop early if start_offset > pc_offset, right?

https://codereview.chromium.org/1010883002/diff/40001/src/objects.cc#newcode8547
src/objects.cc:8547: if (start_offset < innermost_start) continue;
This assumes that for nested try-catch, the start_offset of the inner try-catch
is always larger than the start_offset of the outer one. Is this guaranteed for
all platforms?

[Michael Starzinger, 2015-03-18 10:42:35]

Thanks for the comments. Left some replies.

https://codereview.chromium.org/1010883002/diff/40001/src/objects-inl.h
File src/objects-inl.h (right):

https://codereview.chromium.org/1010883002/diff/40001/src/objects-inl.h#newco...
src/objects-inl.h:767: // a handler table array.
On 2015/03/17 10:21:22, Yang wrote:
> Maybe we want to introduce a special fixed array map? Then again that would
just
> be for debugging and assertions.

Acknowledged. As discussed offline, Benedikt and I were thinking about moving
this table into the code object itself at some point. So I would vote for having
the table be minimally intrusive for now, if you are fine with that.

https://codereview.chromium.org/1010883002/diff/40001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/1010883002/diff/40001/src/objects.cc#newcode8540
src/objects.cc:8540: for (int i = 0; i < length(); i += kRangeEntrySize) {
On 2015/03/17 10:21:22, Yang wrote:
> The handler table should be sorted by start_offset (which we could also
assert).
> We could end the loop early if start_offset > pc_offset, right?

Acknowledged. Unfortunately the start offsets are not strictly increasing
because the handler index is given out by the parser in "post parse tree"-ish
order whereas the code is generated in "pre AST"-ish order. So for nested
try-catch or try-finally statement the order is almost guaranteed to be screwed
up. I am totally up for optimizing this later once we have measurements of the
overhead that these lookup operations incur.

https://codereview.chromium.org/1010883002/diff/40001/src/objects.cc#newcode8547
src/objects.cc:8547: if (start_offset < innermost_start) continue;
On 2015/03/17 10:21:22, Yang wrote:
> This assumes that for nested try-catch, the start_offset of the inner
try-catch
> is always larger than the start_offset of the outer one. Is this guaranteed
for
> all platforms?

Acknowledged. Yes, this should hold on all architectures, because every nested
try-catch or try-finally statement will at least push the context register
before it enters the try-block. The DCHECK one line above tries to guard that.
Let me know if you can think of a better way to guard that, or particular test
cases that we should add.

As per offline discussion, there are tests in mjsunit/try.js that have tightly
nested try-blocks. I verified that there is one where the start offsets are as
close together as possible (i.e. return_from_nested_catch) and one where the end
offsets are as close together as possible (i.e. break_from_nested_catch)
already.

https://codereview.chromium.org/1010883002/diff/40001/src/runtime/runtime-gen...
File src/runtime/runtime-generator.cc (right):

https://codereview.chromium.org/1010883002/diff/40001/src/runtime/runtime-gen...
src/runtime/runtime-generator.cc:70: }
On 2015/03/17 09:41:26, wingo wrote:
> Hoo, nasty stuff.  ACK though :)

Acknowledged. Yeah, it's nasty. But my campaign against full-codegen keeping
track of the operand stack height at expression level will find it's end once
do-expressions become reality. Because then even I would agree that keeping
track of stack height statically is unavoidable. Best of luck to the poor soul
having to implement that, I hope it won't be me. :)

[Michael Starzinger, 2015-03-19 09:44:52]

Patchset #4 (id:60001) has been deleted

[Michael Starzinger, 2015-03-24 09:08:30]

The CQ bit was checked by mstarzinger@chromium.org

[Michael Starzinger, 2015-03-24 09:08:31]

The patchset sent to the CQ was uploaded after l-g-t-m from wingo@igalia.com,
yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/1010883002/#ps180001
(title: "Follow-up fixes.")

[commit-bot: I haz the power, 2015-03-24 09:08:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1010883002/180001

[commit-bot: I haz the power, 2015-03-24 10:18:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-03-24 10:18:38]

Try jobs failed on following builders:
  v8_linux_arm_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel/builds/2696)

[Michael Starzinger, 2015-03-24 16:10:40]

Akos, Paul: Just a quick heads up, I plan to land this tomorrow.

[Michael Starzinger, 2015-03-25 13:11:34]

The CQ bit was checked by mstarzinger@chromium.org

[Michael Starzinger, 2015-03-25 13:11:35]

The patchset sent to the CQ was uploaded after l-g-t-m from wingo@igalia.com,
yangguo@chromium.org
Link to the patchset: https://codereview.chromium.org/1010883002/#ps220001
(title: "Fix debugger-pause-on-promise-rejection.")

[commit-bot: I haz the power, 2015-03-25 13:11:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1010883002/220001

[commit-bot: I haz the power, 2015-03-25 13:13:58]

Committed patchset #11 (id:220001)

[commit-bot: I haz the power, 2015-03-25 13:14:15]

Patchset 11 (id:??) landed as
https://crrev.com/38a719f965d0a83ddac04392d5b9c5abe214281c
Cr-Commit-Position: refs/heads/master@{#27440}