Revert remove dead hole check logic

Revert remove dead hole check logic

This reverts commits 
* 45c11887929c1f0fbc54849d2627cd8bb90bcc37 "[fullcodegen] Remove dead hole check logic"
* 2aaf217bd05677c9dc8cab7242f698836a830b0a "[crankshaft] Remove dead Variable hole-checking code"
* f40a3817e12b08a0d206f4f4c6887c141ec67085 "[crankshaft] Fix mips/mips64 build: remove unused variable"



BUG=
R=adamk@chromium.org,bmeurer@chromium.org,hablich@chromium.org

Review-Url: https://codereview.chromium.org/2640793004
Cr-Commit-Position: refs/heads/master@{#42476}
Committed: https://chromium.googlesource.com/v8/v8/+/80234ee31f7387036f91b72cce8893300ada5657

[ofrobots, 2017-01-18 15:36:40]

Description was changed from

==========
Revert "[fullcodegen] Remove dead hole check logic"

This reverts commit 45c11887929c1f0fbc54849d2627cd8bb90bcc37.

This together with 5529430 need to be reverted to fix a regression
with Node.js.

BUG=
R=adamk@chromium.org,bmeurer@chromium.org,mhablich@chromium.org
==========

to

==========
Revert "[fullcodegen] Remove dead hole check logic"

This reverts commit 45c11887929c1f0fbc54849d2627cd8bb90bcc37.

This together with 5529430 need to be reverted to fix a regression
with Node.js.

BUG=
R=adamk@chromium.org,bmeurer@chromium.org,hablich@chromium.org
==========

[ofrobots, 2017-01-18 15:36:40]

ofrobots@google.com changed reviewers:
+ hablich@chromium.org
- mhablich@chromium.org

[ofrobots, 2017-01-18 15:48:52]

The CQ bit was checked by ofrobots@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 15:48:55]

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

[ofrobots, 2017-01-18 15:50:43]

PTAL

[commit-bot: I haz the power, 2017-01-18 16:15:23]

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

[commit-bot: I haz the power, 2017-01-18 16:15:24]

Dry run: This issue passed the CQ dry run.

[adamk, 2017-01-18 17:30:37]

lgtm, but you'll need bmeurer for OWNERS.

Was this sufficient by itself? I would expect you might also have to revert
https://codereview.chromium.org/2622783002 (to bring back hole-checking in
Crankshaft) and https://codereview.chromium.org/2621123002 (a build fix landed
after that one).

[ofrobots, 2017-01-18 17:53:25]

On 2017/01/18 17:30:37, adamk wrote:
> lgtm, but you'll need bmeurer for OWNERS.
> 
> Was this sufficient by itself? I would expect you might also have to revert
> https://codereview.chromium.org/2622783002 (to bring back hole-checking in
> Crankshaft) and https://codereview.chromium.org/2621123002 (a build fix landed
> after that one).

It seems sufficient to fix the Node.js regressions, and the CQ dry-run above (do
you know what's happening with the sanitizer_coverage?). I can add more changes
to the revert, but I don't have the full context on what all *should* be
reverted. Do you think I should add the above two commits?

[Benedikt Meurer, 2017-01-18 17:56:04]

On 2017/01/18 17:53:25, ofrobots wrote:
> On 2017/01/18 17:30:37, adamk wrote:
> > lgtm, but you'll need bmeurer for OWNERS.
> > 
> > Was this sufficient by itself? I would expect you might also have to revert
> > https://codereview.chromium.org/2622783002 (to bring back hole-checking in
> > Crankshaft) and https://codereview.chromium.org/2621123002 (a build fix
landed
> > after that one).
> 
> It seems sufficient to fix the Node.js regressions, and the CQ dry-run above
(do
> you know what's happening with the sanitizer_coverage?). I can add more
changes
> to the revert, but I don't have the full context on what all *should* be
> reverted. Do you think I should add the above two commits?

LGTM, but I agree with Adam that we should also revert the Crankshaft changes to
be sure. Our test coverage for Crankshaft is everything but great, so let's just
assume that you need the hole-check code there.

[ofrobots, 2017-01-18 18:51:53]

Description was changed from

==========
Revert "[fullcodegen] Remove dead hole check logic"

This reverts commit 45c11887929c1f0fbc54849d2627cd8bb90bcc37.

This together with 5529430 need to be reverted to fix a regression
with Node.js.

BUG=
R=adamk@chromium.org,bmeurer@chromium.org,hablich@chromium.org
==========

to

==========
Revert remove dead hole check logic

This reverts commits 
* 45c11887929c1f0fbc54849d2627cd8bb90bcc37 "[fullcodegen] Remove dead hole check
logic"
* 2aaf217bd05677c9dc8cab7242f698836a830b0a "[crankshaft] Remove dead Variable
hole-checking code"
* f40a3817e12b08a0d206f4f4c6887c141ec67085 "[crankshaft] Fix mips/mips64 build:
remove unused variable"



BUG=
R=adamk@chromium.org,bmeurer@chromium.org,hablich@chromium.org
==========

[ofrobots, 2017-01-18 18:52:15]

On 2017/01/18 17:56:04, Benedikt Meurer wrote:
> On 2017/01/18 17:53:25, ofrobots wrote:
> > On 2017/01/18 17:30:37, adamk wrote:
> > > lgtm, but you'll need bmeurer for OWNERS.
> > > 
> > > Was this sufficient by itself? I would expect you might also have to
revert
> > > https://codereview.chromium.org/2622783002 (to bring back hole-checking in
> > > Crankshaft) and https://codereview.chromium.org/2621123002 (a build fix
> landed
> > > after that one).
> > 
> > It seems sufficient to fix the Node.js regressions, and the CQ dry-run above
> (do
> > you know what's happening with the sanitizer_coverage?). I can add more
> changes
> > to the revert, but I don't have the full context on what all *should* be
> > reverted. Do you think I should add the above two commits?
> 
> LGTM, but I agree with Adam that we should also revert the Crankshaft changes
to
> be sure. Our test coverage for Crankshaft is everything but great, so let's
just
> assume that you need the hole-check code there.

Added the other two reverts. PTAL.

[ofrobots, 2017-01-18 18:52:52]

The CQ bit was checked by ofrobots@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 18:53:02]

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

[adamk, 2017-01-18 18:56:07]

lgtm

[Benedikt Meurer, 2017-01-18 19:05:08]

LGTM!

[commit-bot: I haz the power, 2017-01-18 19:24:50]

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

[commit-bot: I haz the power, 2017-01-18 19:24:51]

Dry run: This issue passed the CQ dry run.

[ofrobots, 2017-01-18 19:25:16]

The CQ bit was checked by ofrobots@google.com

[commit-bot: I haz the power, 2017-01-18 19:25:21]

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, 2017-01-18 19:27:42]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484767516470410,
"parent_rev": "0ecc6b060045d617ac31d94816054d2a3cca7855", "commit_rev":
"80234ee31f7387036f91b72cce8893300ada5657"}

[commit-bot: I haz the power, 2017-01-18 19:27:48]

Description was changed from

==========
Revert remove dead hole check logic

This reverts commits 
* 45c11887929c1f0fbc54849d2627cd8bb90bcc37 "[fullcodegen] Remove dead hole check
logic"
* 2aaf217bd05677c9dc8cab7242f698836a830b0a "[crankshaft] Remove dead Variable
hole-checking code"
* f40a3817e12b08a0d206f4f4c6887c141ec67085 "[crankshaft] Fix mips/mips64 build:
remove unused variable"



BUG=
R=adamk@chromium.org,bmeurer@chromium.org,hablich@chromium.org
==========

to

==========
Revert remove dead hole check logic

This reverts commits 
* 45c11887929c1f0fbc54849d2627cd8bb90bcc37 "[fullcodegen] Remove dead hole check
logic"
* 2aaf217bd05677c9dc8cab7242f698836a830b0a "[crankshaft] Remove dead Variable
hole-checking code"
* f40a3817e12b08a0d206f4f4c6887c141ec67085 "[crankshaft] Fix mips/mips64 build:
remove unused variable"



BUG=
R=adamk@chromium.org,bmeurer@chromium.org,hablich@chromium.org

Review-Url: https://codereview.chromium.org/2640793004
Cr-Commit-Position: refs/heads/master@{#42476}
Committed:
https://chromium.googlesource.com/v8/v8/+/80234ee31f7387036f91b72cce8893300ad...
==========

[commit-bot: I haz the power, 2017-01-18 19:27:49]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/v8/v8/+/80234ee31f7387036f91b72cce8893300ad...