[stubs] Enable graph verification for builtins.

[stubs] Enable graph verification for builtins.

... and fix the inconsistencies.

BUG=

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

[Igor Sheludko, 2016-12-13 17:34:03]

Patchset #2 (id:20001) has been deleted

[Igor Sheludko, 2016-12-13 17:34:46]

Patchset #1 (id:1) has been deleted

[Igor Sheludko, 2016-12-13 17:37:02]

Description was changed from

==========
[stubs] Enable CSA graph verification in debug mode.

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

to

==========
[stubs] Enable graph verification for builtins.

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

[Igor Sheludko, 2016-12-13 18:17:04]

Patchset #1 (id:40001) has been deleted

[Igor Sheludko, 2016-12-13 18:32:03]

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

[commit-bot: I haz the power, 2016-12-13 18:32:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 19:00:04]

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

[commit-bot: I haz the power, 2016-12-13 19:00:05]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-12-14 08:53:59]

Description was changed from

==========
[stubs] Enable graph verification for builtins.

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

to

==========
[stubs] Enable graph verification for builtins.

... and fix the inconsistencies.

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

[Igor Sheludko, 2016-12-14 08:54:51]

Patchset #2 (id:80001) has been deleted

[Igor Sheludko, 2016-12-14 08:58:09]

Patchset #1 (id:60001) has been deleted

[Igor Sheludko, 2016-12-14 08:59:36]

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

[commit-bot: I haz the power, 2016-12-14 08:59:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 09:41:49]

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

[commit-bot: I haz the power, 2016-12-14 09:41:50]

Dry run: This issue passed the CQ dry run.

[Igor Sheludko, 2016-12-14 10:16:18]

ishell@chromium.org changed reviewers:
+ epertoso@chromium.org

[Igor Sheludko, 2016-12-14 10:16:18]

PTAL

[epertoso, 2016-12-14 10:34:33]

lgtm

[Igor Sheludko, 2016-12-14 10:48:52]

The CQ bit was checked by ishell@chromium.org

[commit-bot: I haz the power, 2016-12-14 10:49:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 10:51:01]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1481712532857330,
"parent_rev": "326809898b98474383dd3b6652e2a3712a9c2f28", "commit_rev":
"8268f341c917551723d75fceb720262ae6c4a499"}

[commit-bot: I haz the power, 2016-12-14 10:51:07]

Description was changed from

==========
[stubs] Enable graph verification for builtins.

... and fix the inconsistencies.

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

to

==========
[stubs] Enable graph verification for builtins.

... and fix the inconsistencies.

BUG=

Review-Url: https://codereview.chromium.org/2573573004
==========

[commit-bot: I haz the power, 2016-12-14 10:51:08]

Committed patchset #1 (id:100001)

[commit-bot: I haz the power, 2016-12-14 10:51:35]

Description was changed from

==========
[stubs] Enable graph verification for builtins.

... and fix the inconsistencies.

BUG=

Review-Url: https://codereview.chromium.org/2573573004
==========

to

==========
[stubs] Enable graph verification for builtins.

... and fix the inconsistencies.

BUG=

Committed: https://crrev.com/a54d7acb11c6fd246a39d1b8945d144de9e9d7c6
Cr-Commit-Position: refs/heads/master@{#41690}
==========

[commit-bot: I haz the power, 2016-12-14 10:51:36]

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

[Nico, 2017-09-15 19:28:23]

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

[Nico, 2017-09-15 19:28:27]

https://codereview.chromium.org/2573573004/diff/100001/src/compiler/machine-g...
File src/compiler/machine-graph-verifier.cc (right):

https://codereview.chromium.org/2573573004/diff/100001/src/compiler/machine-g...
src/compiler/machine-graph-verifier.cc:511: break;
This break is probably supposed to go in the default branch instead of after the
switch? As-is, the code looks like

  for (...) {
    switch (...) {}
    break;
  }

i.e. the for loop runs exactly once, with i = 0.

(found by this warning, which I'm evaluating:

../../v8/src/compiler/machine-graph-verifier.cc(535,50):  error: loop will run
at most once (loop increment never executed)
[-Werror,-Wunreachable-code-loop-increment]
            for (size_t i = 0; i < return_count; i++) {
                                                 ^~~
)

[Igor Sheludko, 2017-09-20 15:15:20]

https://codereview.chromium.org/2573573004/diff/100001/src/compiler/machine-g...
File src/compiler/machine-graph-verifier.cc (right):

https://codereview.chromium.org/2573573004/diff/100001/src/compiler/machine-g...
src/compiler/machine-graph-verifier.cc:511: break;
On 2017/09/15 19:28:25, Nico wrote:
> This break is probably supposed to go in the default branch instead of after
the
> switch? As-is, the code looks like
> 
>   for (...) {
>     switch (...) {}
>     break;
>   }
> 
> i.e. the for loop runs exactly once, with i = 0.
> 
> (found by this warning, which I'm evaluating:
> 
> ../../v8/src/compiler/machine-graph-verifier.cc(535,50):  error: loop will run
> at most once (loop increment never executed)
> [-Werror,-Wunreachable-code-loop-increment]
>             for (size_t i = 0; i < return_count; i++) {
>                                                  ^~~
> )

Nice catch! Thank you!
The fix: https://chromium-review.googlesource.com/c/v8/v8/+/674940