Remove value check from ICData checks/house-keeping

Remove value check from ICData checks/house-keeping

BUG=
R=johnmccutchan@google.com, vegorov@google.com

Committed: https://github.com/dart-lang/sdk/commit/91651f88059a971348bfdf01af8549c05b791e91

[srdjan, 2015-05-23 00:13:57]

srdjan@google.com changed reviewers:
+ johnmccutchan@google.com, vegorov@google.com

[srdjan, 2015-05-23 00:13:58]

As suggested by Slava, instead of throttling ICData when excessive value type
polymorphism occurs, remove value-type from checks.
Performance seems unchanged (will test on Golem once I can mount /auto on Mac OS
X).

[Cutch, 2015-05-27 02:53:55]

LGTM with question

https://codereview.chromium.org/1153963002/diff/40001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1153963002/diff/40001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:11742: const intptr_t count = GetCountAt(0);
We are creating a new ICData and filling it with class ids, but, we're reusing
the count from *this? Why?

[Vyacheslav Egorov (Google), 2015-05-27 15:06:35]

lgtm

https://codereview.chromium.org/1153963002/diff/40001/runtime/vm/flow_graph_o...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/1153963002/diff/40001/runtime/vm/flow_graph_o...
runtime/vm/flow_graph_optimizer.cc:1424: GrowableArray<intptr_t> smi_mint_cids;
same TODO as above

https://codereview.chromium.org/1153963002/diff/40001/runtime/vm/flow_graph_o...
runtime/vm/flow_graph_optimizer.cc:1428: return InlineSetIndexed(kind, target,
call, receiver, token_pos,
InlineSetIndexed has an interesting code inside

  if ((array_cid != kTypedDataUint32ArrayCid) &&
      (array_cid != kTypedDataInt32ArrayCid) &&
      !value_check.IsNull()) {

so it seems value_check is not even used and there is no reason to populate it.

https://codereview.chromium.org/1153963002/diff/40001/runtime/vm/flow_graph_o...
runtime/vm/flow_graph_optimizer.cc:4549: needs_store_barrier,
inline needs_store_barrier value here as it's always kEmitStoreBarrier?

[srdjan, 2015-05-27 19:10:58]

https://codereview.chromium.org/1153963002/diff/40001/runtime/vm/flow_graph_o...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/1153963002/diff/40001/runtime/vm/flow_graph_o...
runtime/vm/flow_graph_optimizer.cc:1424: GrowableArray<intptr_t> smi_mint_cids;
On 2015/05/27 15:06:34, Vyacheslav Egorov (Google) wrote:
> same TODO as above

Done.

https://codereview.chromium.org/1153963002/diff/40001/runtime/vm/flow_graph_o...
runtime/vm/flow_graph_optimizer.cc:1428: return InlineSetIndexed(kind, target,
call, receiver, token_pos,
On 2015/05/27 15:06:34, Vyacheslav Egorov (Google) wrote:
> InlineSetIndexed has an interesting code inside
> 
>   if ((array_cid != kTypedDataUint32ArrayCid) &&
>       (array_cid != kTypedDataInt32ArrayCid) &&
>       !value_check.IsNull()) {
> 
> so it seems value_check is not even used and there is no reason to populate
it.

Thanks! Also removed AsUnaryClassChecksForCids

https://codereview.chromium.org/1153963002/diff/40001/runtime/vm/flow_graph_o...
runtime/vm/flow_graph_optimizer.cc:4549: needs_store_barrier,
On 2015/05/27 15:06:34, Vyacheslav Egorov (Google) wrote:
> inline needs_store_barrier value here as it's always kEmitStoreBarrier?

Done.

https://codereview.chromium.org/1153963002/diff/40001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1153963002/diff/40001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:11742: const intptr_t count = GetCountAt(0);
On 2015/05/27 02:53:55, Cutch wrote:
> We are creating a new ICData and filling it with class ids, but, we're reusing
> the count from *this? Why?

The count is interesting as as it means different things if it is 0 or > 0. By
copying it we can pass that information to the newly generated ICData. In
current use (value checks) it does not matter, but IMO, it makes it more robust
if we pass that information.

Added comment.

[srdjan, 2015-05-27 20:17:47]

Committed patchset #4 (id:60001) manually as
91651f88059a971348bfdf01af8549c05b791e91 (presubmit successful).