Reland [heap] Avoid the use of cells to point from code to new-space objects.

Reland [heap] Avoid the use of cells to point from code to new-space objects.

The reason for reverting was: [Sheriff] Breaks arm debug:
https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm%20-%20sim%20-%20debug/builds/1038.

The problem was the dereferencing of handles for smi checks. It turned out
that these smi checks can be removed anyways, both on arm and on mips.

Additionally some rebasing was necessary.

Original issue's description:

Cells were needed originally because there was no typed remembered set to
record direct pointers from code space to new space. A previous
CL (https://codereview.chromium.org/2003553002/) already introduced
the remembered set, this CL uses it.

This CL
* stores direct pointers in code objects, even if the target is in new space,
* records the slot of the pointer in typed-old-to-new remembered set,
* adds a list which stores weak code-to-new-space references,
* adds a test to test-heap.cc for weak code-to-new-space references,
* removes prints in tail-call-megatest.js

R=mlippautz@chromium.org

Committed: https://crrev.com/5508e16592522658587da71ba6743c8e832fe4d1
Cr-Commit-Position: refs/heads/master@{#37217}

[ahaas, 2016-06-23 11:44:52]

PTAL

[Michael Lippautz, 2016-06-23 11:51:12]

lgtm

Canary seems like a sad place to be at the moment so think twice about landing
it immediately

https://codereview.chromium.org/2091733002/diff/20001/src/heap/remembered-set.cc
File src/heap/remembered-set.cc (right):

https://codereview.chromium.org/2091733002/diff/20001/src/heap/remembered-set...
src/heap/remembered-set.cc:22: {
nit: I too like building pyramids but there's no need for this additional scopes
;)

https://codereview.chromium.org/2091733002/diff/20001/src/heap/remembered-set...
src/heap/remembered-set.cc:32: }
nit: You can also get rid of the outer scopes, so it could be
STATIC_ASSERT
for (...) {
...
}
for (...) {
...
}

[ahaas, 2016-06-23 12:42:21]

https://codereview.chromium.org/2091733002/diff/20001/src/heap/remembered-set.cc
File src/heap/remembered-set.cc (right):

https://codereview.chromium.org/2091733002/diff/20001/src/heap/remembered-set...
src/heap/remembered-set.cc:22: {
On 2016/06/23 at 11:51:12, Michael Lippautz wrote:
> nit: I too like building pyramids but there's no need for this additional
scopes ;)

done.

https://codereview.chromium.org/2091733002/diff/20001/src/heap/remembered-set...
src/heap/remembered-set.cc:32: }
On 2016/06/23 at 11:51:12, Michael Lippautz wrote:
> nit: You can also get rid of the outer scopes, so it could be
> STATIC_ASSERT
> for (...) {
> ...
> }
> for (...) {
> ...
> }

done.

[ahaas, 2016-06-23 12:43:11]

The CQ bit was checked by ahaas@chromium.org

[ahaas, 2016-06-23 12:43:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlippautz@chromium.org
Link to the patchset: https://codereview.chromium.org/2091733002/#ps40001
(title: "Address comments.")

[commit-bot: I haz the power, 2016-06-23 12:43:23]

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

[commit-bot: I haz the power, 2016-06-23 13:11:40]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-23 13:14:28]

Description was changed from

==========
Reland [heap] Avoid the use of cells to point from code to new-space objects.

The reason for reverting was: [Sheriff] Breaks arm debug:
https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm%20....

The problem was the dereferencing of handles for smi checks. It turned out
that these smi checks can be removed anyways, both on arm and on mips.

Additionally some rebasing was necessary.

Original issue's description:

Cells were needed originally because there was no typed remembered set to
record direct pointers from code space to new space. A previous
CL (https://codereview.chromium.org/2003553002/) already introduced
the remembered set, this CL uses it.

This CL
* stores direct pointers in code objects, even if the target is in new space,
* records the slot of the pointer in typed-old-to-new remembered set,
* adds a list which stores weak code-to-new-space references,
* adds a test to test-heap.cc for weak code-to-new-space references,
* removes prints in tail-call-megatest.js

R=mlippautz@chromium.org
==========

to

==========
Reland [heap] Avoid the use of cells to point from code to new-space objects.

The reason for reverting was: [Sheriff] Breaks arm debug:
https://build.chromium.org/p/client.v8.ports/builders/V8%20Linux%20-%20arm%20....

The problem was the dereferencing of handles for smi checks. It turned out
that these smi checks can be removed anyways, both on arm and on mips.

Additionally some rebasing was necessary.

Original issue's description:

Cells were needed originally because there was no typed remembered set to
record direct pointers from code space to new space. A previous
CL (https://codereview.chromium.org/2003553002/) already introduced
the remembered set, this CL uses it.

This CL
* stores direct pointers in code objects, even if the target is in new space,
* records the slot of the pointer in typed-old-to-new remembered set,
* adds a list which stores weak code-to-new-space references,
* adds a test to test-heap.cc for weak code-to-new-space references,
* removes prints in tail-call-megatest.js

R=mlippautz@chromium.org

Committed: https://crrev.com/5508e16592522658587da71ba6743c8e832fe4d1
Cr-Commit-Position: refs/heads/master@{#37217}
==========

[commit-bot: I haz the power, 2016-06-23 13:14:30]

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

[vogelheim, 2016-06-23 16:02:09]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2090983002/ by vogelheim@chromium.org.

The reason for reverting is: This breaks gc-stress bot:
https://chromegw.corp.google.com/i/client.v8/builders/V8%20Linux64%20GC%20Str...

#
# Fatal error in ../../src/heap/mark-compact.cc, line 3715
# Check failed:
Page::FromAddress(reinterpret_cast<HeapObject*>(*slot)->address())
->IsFlagSet(Page::PAGE_NEW_NEW_PROMOTION).
#


I can reproduce locally, and local revert also fixes it -> revert.

Reproduce with:
 out/Debug/d8 --test --random-seed=2140216864 --nohard-abort
--nodead-code-elimination --nofold-constants --enable-slow-asserts --debug-code
--verify-heap --allow-natives-syntax --harmony-tailcalls test/mjsunit/mjsunit.js
 test/mjsunit/es6/tail-call-megatest-shard2.js --gc-interval=500
--stress-compaction --concurrent-recompilation-queue-length=64
--concurrent-recompilation-delay=500 --concurrent-recompilation

(Maybe run in loop; it's flaky when broken; but passes reliably w/ revert.).

[Michael Lippautz, 2016-06-23 16:08:19]

On 2016/06/23 16:02:09, vogelheim wrote:
> A revert of this CL (patchset #3 id:40001) has been created in
> https://codereview.chromium.org/2090983002/ by mailto:vogelheim@chromium.org.
> 
> The reason for reverting is: This breaks gc-stress bot:
>
https://chromegw.corp.google.com/i/client.v8/builders/V8%20Linux64%20GC%20Str...
> 
> #
> # Fatal error in ../../src/heap/mark-compact.cc, line 3715
> # Check failed:
> Page::FromAddress(reinterpret_cast<HeapObject*>(*slot)->address())
> ->IsFlagSet(Page::PAGE_NEW_NEW_PROMOTION).
> #
> 
> 
> I can reproduce locally, and local revert also fixes it -> revert.
> 
> Reproduce with:
>  out/Debug/d8 --test --random-seed=2140216864 --nohard-abort
> --nodead-code-elimination --nofold-constants --enable-slow-asserts
--debug-code
> --verify-heap --allow-natives-syntax --harmony-tailcalls
test/mjsunit/mjsunit.js
>  test/mjsunit/es6/tail-call-megatest-shard2.js --gc-interval=500
> --stress-compaction --concurrent-recompilation-queue-length=64
> --concurrent-recompilation-delay=500 --concurrent-recompilation
> 
> (Maybe run in loop; it's flaky when broken; but passes reliably w/ revert.).

FYI Andreas: This looks like a missed WB as you basically have a slot pointing
to *to* space during slot iteration. This can only happen when pages move within
new space.  (The branch here is pretty new; previously you would've run into the
dcheck right below:  DCHECK(!heap->InNewSpace(*slot));)