VectorICs: allocating slots for store ics in ast nodes.

VectorICs: allocating slots for store ics in ast nodes.

Also adapt code generation to pass the slot to the
store/keyed-store ic. AST nodes ObjectLiteral, Assignment,
ForEach, Call and CountOperation now include one or more
feedback vector ic slot ids.

BUG=

Committed: https://crrev.com/5450fc07ba07615a70f5ed8379dc23c3275d6fe3
Cr-Commit-Position: refs/heads/master@{#28659}

[mvstanton, 2015-05-27 09:52:41]

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

[mvstanton, 2015-05-27 09:54:21]

Hi Jakob,
The next in a fun series. This CL allocates the necessary slots for the stores
ICs, and alters full-codegen to pass them appropriately.

Still to be done is turbofan and crankshaft codegen, and handling vector-based
misses in ic.cc.

Ports to follow...
--Michael

[Jakob Kummerow, 2015-05-27 13:24:36]

LGTM with comments.

https://codereview.chromium.org/1161623002/diff/50001/src/ast.cc
File src/ast.cc (right):

https://codereview.chromium.org/1161623002/diff/50001/src/ast.cc#newcode316
src/ast.cc:316: 
As discussed, this is kind of brittle, but hard to avoid. Two suggestions:
(1) Add a comment here stating that the logic must mirror
FullCodeGenerator::VisitObjectLiteral (so readers of the code know where to
look).
(2) Add a (D)CHECK that FullCodeGenerator::VisitObjectLiteral consumed exactly
as many slots as were allocated here. This would also be the natural place for a
comment clarifying that the number of slots is computed here.

https://codereview.chromium.org/1161623002/diff/50001/src/ast.cc#newcode347
src/ast.cc:347: return GetNthSlot(*slot_index++);
I'm pretty sure this moves the pointer instead of incrementing the value it
points to... either way, I prefer explicit parentheses, i.e.:

    return GetNthSlot((*slot_index)++);

https://codereview.chromium.org/1161623002/diff/50001/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/1161623002/diff/50001/src/ast.h#newcode799
src/ast.h:799: // Object literals need one feedback slot for each non-trivial
value, as well
Object literals? Huh?

[mvstanton, 2015-05-27 14:00:19]

Thanks for the review Jakob, I addressed your comments. All the best,
--Michael

https://codereview.chromium.org/1161623002/diff/50001/src/ast.cc
File src/ast.cc (right):

https://codereview.chromium.org/1161623002/diff/50001/src/ast.cc#newcode316
src/ast.cc:316: 
On 2015/05/27 13:24:35, Jakob wrote:
> As discussed, this is kind of brittle, but hard to avoid. Two suggestions:
> (1) Add a comment here stating that the logic must mirror
> FullCodeGenerator::VisitObjectLiteral (so readers of the code know where to
> look).
> (2) Add a (D)CHECK that FullCodeGenerator::VisitObjectLiteral consumed exactly
> as many slots as were allocated here. This would also be the natural place for
a
> comment clarifying that the number of slots is computed here.

Good idea, done.

https://codereview.chromium.org/1161623002/diff/50001/src/ast.cc#newcode347
src/ast.cc:347: return GetNthSlot(*slot_index++);
On 2015/05/27 13:24:35, Jakob wrote:
> I'm pretty sure this moves the pointer instead of incrementing the value it
> points to... either way, I prefer explicit parentheses, i.e.:
> 
>     return GetNthSlot((*slot_index)++);

Heh-heh. :p Fixed, thanks!

https://codereview.chromium.org/1161623002/diff/50001/src/ast.h
File src/ast.h (right):

https://codereview.chromium.org/1161623002/diff/50001/src/ast.h#newcode799
src/ast.h:799: // Object literals need one feedback slot for each non-trivial
value, as well
On 2015/05/27 13:24:36, Jakob wrote:
> Object literals? Huh?

Doh...copy/paste. Removed.

[mvstanton, 2015-05-27 14:00:57]

The CQ bit was checked by mvstanton@chromium.org

[mvstanton, 2015-05-27 14:00:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
jkummerow@chromium.org
Link to the patchset: https://codereview.chromium.org/1161623002/#ps70001
(title: "Code comments.")

[commit-bot: I haz the power, 2015-05-27 14:01:12]

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

[commit-bot: I haz the power, 2015-05-27 14:26:17]

Committed patchset #5 (id:70001)

[commit-bot: I haz the power, 2015-05-27 14:26:34]

Patchset 5 (id:??) landed as
https://crrev.com/5450fc07ba07615a70f5ed8379dc23c3275d6fe3
Cr-Commit-Position: refs/heads/master@{#28659}