Assign yield ids in ast-numbering rather than in bytecode-generator.

[generators] Assign yield ids in ast-numbering rather than in bytecode-generator.

This avoids weird control flow when the bytecode generator skips dead code containing yields.

BUG=v8:4907
LOG=n

Committed: https://crrev.com/21f2c3eafe335de591a1c97967cfc89138dd7a55
Cr-Commit-Position: refs/heads/master@{#35940}

[neis, 2016-04-28 15:40:28]

The CQ bit was checked by neis@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-28 15:40:39]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1927943003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1927943003/1

[neis, 2016-04-28 15:45:17]

Description was changed from

==========
Assign yield ids in ast-numbering rather than in bytecode-generator.

BUG=
==========

to

==========
[generators] Assign yield ids in ast-numbering rather than in
bytecode-generator.

This avoids weird control flow when the bytecode generator skips dead code
containing yields.

BUG=v8:4907
LOG=n
==========

[neis, 2016-04-28 15:49:31]

neis@chromium.org changed reviewers:
+ jarin@chromium.org, rmcilroy@chromium.org

[neis, 2016-04-28 15:49:47]

PTAL

[commit-bot: I haz the power, 2016-04-28 16:06:40]

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

[commit-bot: I haz the power, 2016-04-28 16:06:41]

Dry run: This issue passed the CQ dry run.

[Jarin, 2016-04-28 16:31:18]

https://codereview.chromium.org/1927943003/diff/1/src/ast/ast.h
File src/ast/ast.h (right):

https://codereview.chromium.org/1927943003/diff/1/src/ast/ast.h#newcode2540
src/ast/ast.h:2540: int id() const { return id_; }
I believe the Expression class already has an id() method for bailouts. Please,
do not overload it. (I am actually a bit surprised that the compiler does not
warn about this.)

https://codereview.chromium.org/1927943003/diff/1/src/interpreter/bytecode-ge...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/1927943003/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:669: size_t first =
stmt->first_yield_id();
I am mildly surprised this works is not first_yield_id an integer?

[neis, 2016-04-29 07:52:24]

https://codereview.chromium.org/1927943003/diff/1/src/ast/ast.h
File src/ast/ast.h (right):

https://codereview.chromium.org/1927943003/diff/1/src/ast/ast.h#newcode2540
src/ast/ast.h:2540: int id() const { return id_; }
On 2016/04/28 16:31:18, Jarin wrote:
> I believe the Expression class already has an id() method for bailouts.
Please,
> do not overload it. (I am actually a bit surprised that the compiler does not
> warn about this.)

Oh indeed. Done.

https://codereview.chromium.org/1927943003/diff/1/src/interpreter/bytecode-ge...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/1927943003/diff/1/src/interpreter/bytecode-ge...
src/interpreter/bytecode-generator.cc:669: size_t first =
stmt->first_yield_id();
On 2016/04/28 16:31:18, Jarin wrote:
> I am mildly surprised this works is not first_yield_id an integer?

signed to unsigned is always "fine" as far as I know

[rmcilroy, 2016-04-29 08:31:43]

Nice. Interpreter changes LGTM.

[neis, 2016-05-02 14:20:40]

neis@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[neis, 2016-05-02 14:20:41]

Michi, PTAL

[Michael Starzinger, 2016-05-02 14:31:16]

LGTM.

https://codereview.chromium.org/1927943003/diff/20001/src/interpreter/bytecod...
File src/interpreter/bytecode-generator.cc (right):

https://codereview.chromium.org/1927943003/diff/20001/src/interpreter/bytecod...
src/interpreter/bytecode-generator.cc:669: size_t first =
stmt->first_yield_id();
nit: s/first/first_yield/

[neis, 2016-05-02 14:48:11]

The CQ bit was checked by neis@chromium.org

[neis, 2016-05-02 14:48:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
rmcilroy@chromium.org, mstarzinger@chromium.org
Link to the patchset: https://codereview.chromium.org/1927943003/#ps40001
(title: "nit")

[commit-bot: I haz the power, 2016-05-02 14:48:13]

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

[commit-bot: I haz the power, 2016-05-02 15:16:40]

Description was changed from

==========
[generators] Assign yield ids in ast-numbering rather than in
bytecode-generator.

This avoids weird control flow when the bytecode generator skips dead code
containing yields.

BUG=v8:4907
LOG=n
==========

to

==========
[generators] Assign yield ids in ast-numbering rather than in
bytecode-generator.

This avoids weird control flow when the bytecode generator skips dead code
containing yields.

BUG=v8:4907
LOG=n
==========

[commit-bot: I haz the power, 2016-05-02 15:16:42]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-02 15:18:20]

Description was changed from

==========
[generators] Assign yield ids in ast-numbering rather than in
bytecode-generator.

This avoids weird control flow when the bytecode generator skips dead code
containing yields.

BUG=v8:4907
LOG=n
==========

to

==========
[generators] Assign yield ids in ast-numbering rather than in
bytecode-generator.

This avoids weird control flow when the bytecode generator skips dead code
containing yields.

BUG=v8:4907
LOG=n

Committed: https://crrev.com/21f2c3eafe335de591a1c97967cfc89138dd7a55
Cr-Commit-Position: refs/heads/master@{#35940}
==========

[commit-bot: I haz the power, 2016-05-02 15:18:21]

Patchset 3 (id:??) landed as
https://crrev.com/21f2c3eafe335de591a1c97967cfc89138dd7a55
Cr-Commit-Position: refs/heads/master@{#35940}