Move delete-last-fast-property code from CSA to C++

Move delete-last-fast-property code from CSA to C++

When deleting the most recently added fast property from an object
by undoing its last map transition, we must clear any recorded slots.
This can only be done in C++, so this functionality must move out
of the stub.
Also update a CHECK in the JSObject verifier to allow backing stores
sticking around after such property deletions.

BUG=chromium:716912, chromium:714981
Review-Url: https://codereview.chromium.org/2854373002
Cr-Commit-Position: refs/heads/master@{#45069}
Committed: https://chromium.googlesource.com/v8/v8/+/6cb995b936d63ce651273ff420d3dbf80ff71f13

[Jakob Kummerow, 2017-05-03 12:26:50]

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

[Jakob Kummerow, 2017-05-03 12:26:50]

PTAL. This is a straightforward port of logic from CSA to C++, with one
addition: recorded slot clearing.

We could also consider simply undoing the original change, with no replacement.
It does yield perf benefits though, so I think it's worth giving it another
chance.

[Igor Sheludko, 2017-05-03 14:39:25]

lgtm with nits:

https://codereview.chromium.org/2854373002/diff/1/src/runtime/runtime-object.cc
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/2854373002/diff/1/src/runtime/runtime-object....
src/runtime/runtime-object.cc:134: if (map->instance_type() <=
LAST_SPECIAL_RECEIVER_TYPE) return false;
map->IsSpecialReceiverMap()

https://codereview.chromium.org/2854373002/diff/1/src/runtime/runtime-object....
src/runtime/runtime-object.cc:158: // (5) The last transition must hvae been
caused by adding a property
s/hvae/have/

https://codereview.chromium.org/2854373002/diff/1/src/runtime/runtime-object....
src/runtime/runtime-object.cc:173: if (index.is_inobject() &&
!map->IsUnboxedDoubleField(index)) {
"index.is_inobject() &&" is not necessary because Map::IsUnboxedDoubleField()
also checks that.

[Jakob Kummerow, 2017-05-03 14:49:27]

Thanks for the review, feedback addressed. I've also done the "is_stable()"
change we discussed offline.

https://codereview.chromium.org/2854373002/diff/1/src/runtime/runtime-object.cc
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/2854373002/diff/1/src/runtime/runtime-object....
src/runtime/runtime-object.cc:134: if (map->instance_type() <=
LAST_SPECIAL_RECEIVER_TYPE) return false;
On 2017/05/03 14:39:25, Igor Sheludko wrote:
> map->IsSpecialReceiverMap()

Done.

https://codereview.chromium.org/2854373002/diff/1/src/runtime/runtime-object....
src/runtime/runtime-object.cc:158: // (5) The last transition must hvae been
caused by adding a property
On 2017/05/03 14:39:25, Igor Sheludko wrote:
> s/hvae/have/

Done.

https://codereview.chromium.org/2854373002/diff/1/src/runtime/runtime-object....
src/runtime/runtime-object.cc:173: if (index.is_inobject() &&
!map->IsUnboxedDoubleField(index)) {
On 2017/05/03 14:39:25, Igor Sheludko wrote:
> "index.is_inobject() &&" is not necessary because Map::IsUnboxedDoubleField()
> also checks that.

Nope, it's necessary. For out-of-object fields, "IsUnboxedDoubleField()" will
return "false" (because it checks for it), but we don't want to enter the
conditional block.

[Jakob Kummerow, 2017-05-03 14:49:35]

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

[commit-bot: I haz the power, 2017-05-03 14:49:42]

Dry run: CQ is trying da patch.

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

[Igor Sheludko, 2017-05-03 14:53:26]

https://codereview.chromium.org/2854373002/diff/1/src/runtime/runtime-object.cc
File src/runtime/runtime-object.cc (right):

https://codereview.chromium.org/2854373002/diff/1/src/runtime/runtime-object....
src/runtime/runtime-object.cc:173: if (index.is_inobject() &&
!map->IsUnboxedDoubleField(index)) {
On 2017/05/03 14:49:27, Jakob Kummerow wrote:
> On 2017/05/03 14:39:25, Igor Sheludko wrote:
> > "index.is_inobject() &&" is not necessary because
Map::IsUnboxedDoubleField()
> > also checks that.
> 
> Nope, it's necessary. For out-of-object fields, "IsUnboxedDoubleField()" will
> return "false" (because it checks for it), but we don't want to enter the
> conditional block.

Acknowledged.

[commit-bot: I haz the power, 2017-05-03 15:18:00]

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

[commit-bot: I haz the power, 2017-05-03 15:18:00]

Dry run: This issue passed the CQ dry run.

[Jakob Kummerow, 2017-05-03 15:49:04]

The CQ bit was checked by jkummerow@chromium.org

[Jakob Kummerow, 2017-05-03 15:49:05]

The patchset sent to the CQ was uploaded after l-g-t-m from ishell@chromium.org
Link to the patchset: https://codereview.chromium.org/2854373002/#ps20001
(title: "addressed comments")

[commit-bot: I haz the power, 2017-05-03 15:49:15]

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-05-03 15:50:51]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493826544829890,
"parent_rev": "1e95840bbf6bfb0eca2789564645de4113c4eacd", "commit_rev":
"6cb995b936d63ce651273ff420d3dbf80ff71f13"}

[commit-bot: I haz the power, 2017-05-03 15:50:55]

Description was changed from

==========
Move delete-last-fast-property code from CSA to C++

When deleting the most recently added fast property from an object
by undoing its last map transition, we must clear any recorded slots.
This can only be done in C++, so this functionality must move out
of the stub.
Also update a CHECK in the JSObject verifier to allow backing stores
sticking around after such property deletions.

BUG=chromium:716912,chromium:714981
==========

to

==========
Move delete-last-fast-property code from CSA to C++

When deleting the most recently added fast property from an object
by undoing its last map transition, we must clear any recorded slots.
This can only be done in C++, so this functionality must move out
of the stub.
Also update a CHECK in the JSObject verifier to allow backing stores
sticking around after such property deletions.

BUG=chromium:716912,chromium:714981

Review-Url: https://codereview.chromium.org/2854373002
Cr-Commit-Position: refs/heads/master@{#45069}
Committed:
https://chromium.googlesource.com/v8/v8/+/6cb995b936d63ce651273ff420d3dbf80ff...
==========

[commit-bot: I haz the power, 2017-05-03 15:50:57]

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