Use weak cell to embed map in CompareNil IC.

Use weak cell to embed map in CompareNil IC.

BUG=v8:3629
LOG=N

Committed: https://crrev.com/31de02151bb409d872fbdc2eedbb3a7ada1d4eb4
Cr-Commit-Position: refs/heads/master@{#25905}

[ulan, 2014-12-18 14:55:29]

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

[ulan, 2014-12-18 14:55:29]

Hi Jakob,

this is the change in BuildCompareNil we discussed. PTAL.

If this sticks, I'll prepare CL that removes special weak-mechanism of
monomorphic IC because all maps will be embedded via WeakCells in IC.

[Jakob Kummerow, 2014-12-18 15:21:07]

Nice, this is easier than I expected. LGTM with nits.

https://codereview.chromium.org/812353002/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/812353002/diff/1/src/hydrogen.cc#newcode3068
src/hydrogen.cc:3068: builder.If<HCompareObjectEqAndBranch>(expected_map, map);
nit: When several IfBuilders are at play, I like naming them by purpose, e.g.
"if_map_matches" or "map_check". Also, the builder sequence can be made shorter
and more readable:

IfBuilder map_check(this);
map_check.IfNot<HCompareObjectEqAndBranch>(expected_map, map);
map_check.ThenDeopt("Unknown map");
map_check.End();

https://codereview.chromium.org/812353002/diff/1/src/hydrogen.cc#newcode3073
src/hydrogen.cc:3073: } else {
nit: add a DCHECK(map_embedding == kEmbedMapsDirectly) as the first line of the
else-block, for documentation.

[ulan, 2014-12-18 15:41:24]

https://codereview.chromium.org/812353002/diff/1/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/812353002/diff/1/src/hydrogen.cc#newcode3068
src/hydrogen.cc:3068: builder.If<HCompareObjectEqAndBranch>(expected_map, map);
On 2014/12/18 15:21:07, Jakob wrote:
> nit: When several IfBuilders are at play, I like naming them by purpose, e.g.
> "if_map_matches" or "map_check". Also, the builder sequence can be made
shorter
> and more readable:
> 
> IfBuilder map_check(this);
> map_check.IfNot<HCompareObjectEqAndBranch>(expected_map, map);
> map_check.ThenDeopt("Unknown map");
> map_check.End();

Done. Thanks, this is much better.

https://codereview.chromium.org/812353002/diff/1/src/hydrogen.cc#newcode3073
src/hydrogen.cc:3073: } else {
On 2014/12/18 15:21:07, Jakob wrote:
> nit: add a DCHECK(map_embedding == kEmbedMapsDirectly) as the first line of
the
> else-block, for documentation.

Done.

[ulan, 2014-12-19 14:23:43]

The CQ bit was checked by ulan@chromium.org

[commit-bot: I haz the power, 2014-12-19 14:24:06]

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

[commit-bot: I haz the power, 2014-12-19 14:51:18]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-12-19 14:51:32]

Patchset 2 (id:??) landed as
https://crrev.com/31de02151bb409d872fbdc2eedbb3a7ada1d4eb4
Cr-Commit-Position: refs/heads/master@{#25905}