Optimize add/set/delete operations for string keys in Maps and Sets

Optimize add/set/delete operations for string keys in Maps and Sets

This was previously landed in commit 8599f5f, but failed the
collections mjsunit test on ia32 with --deopt-every-n=1. The fixed
patch adds a ClearFlag(HValue::kCanOverflow) call after every
arithmetic operation, which should remove all the deopt points in these
intrinsics.

Ideally, it seems like there should be a way to verify that there are
no deopt points for these inline optimized functions, since there's
nothing to deopt to. But I don't currently know of such a thing.

[adamk, 2014-12-05 23:14:27]

adamk@chromium.org changed reviewers:
+ dslomov@chromium.org

[adamk, 2014-12-05 23:14:27]

Okay, this time it should really be fixed. I searched for all numeric operations
and added kCanOverflow flags. Not loving the manual process of that, but at
least I now understand what the failure was.

[Dmitry Lomov (no reviews), 2014-12-05 23:40:59]

On 2014/12/05 23:14:27, adamk wrote:
> Okay, this time it should really be fixed. I searched for all numeric
operations
> and added kCanOverflow flags. Not loving the manual process of that, but at
> least I now understand what the failure was.

Right, I see. I'll take a careful look. How can we verify that all those
operations really do not overflow?

[adamk, 2014-12-05 23:57:14]

On 2014/12/05 23:40:59, Dmitry Lomov (chromium) wrote:
> On 2014/12/05 23:14:27, adamk wrote:
> > Okay, this time it should really be fixed. I searched for all numeric
> operations
> > and added kCanOverflow flags. Not loving the manual process of that, but at
> > least I now understand what the failure was.
> 
> Right, I see. I'll take a careful look. How can we verify that all those
> operations really do not overflow?

Some of them are obvious, like subtracting 1 from the number of buckets (it can
never be < 4).

The ones that aren't so obvious are ones that calculate an element or bucket
index. But my intuiotion is that we'd have run out of memory before any of those
would overflow.

[adamk, 2014-12-06 00:03:49]

On 2014/12/05 23:57:14, adamk wrote:
> On 2014/12/05 23:40:59, Dmitry Lomov (chromium) wrote:
> > On 2014/12/05 23:14:27, adamk wrote:
> > > Okay, this time it should really be fixed. I searched for all numeric
> > operations
> > > and added kCanOverflow flags. Not loving the manual process of that, but
at
> > > least I now understand what the failure was.
> > 
> > Right, I see. I'll take a careful look. How can we verify that all those
> > operations really do not overflow?
> 
> Some of them are obvious, like subtracting 1 from the number of buckets (it
can
> never be < 4).
> 
> The ones that aren't so obvious are ones that calculate an element or bucket
> index. But my intuiotion is that we'd have run out of memory before any of
those
> would overflow.

Any thoughts on the underlying deopt issue? This seems like either a flaw in the
way we write Hydrogen code today or a flaw in the INLINE_OPTIMIZED_FUNCTION
list.

[Dmitry Lomov (no reviews), 2014-12-06 00:14:11]

On 2014/12/06 00:03:49, adamk wrote:
> On 2014/12/05 23:57:14, adamk wrote:
> > On 2014/12/05 23:40:59, Dmitry Lomov (chromium) wrote:
> > > On 2014/12/05 23:14:27, adamk wrote:
> > > > Okay, this time it should really be fixed. I searched for all numeric
> > > operations
> > > > and added kCanOverflow flags. Not loving the manual process of that, but
> at
> > > > least I now understand what the failure was.
> > > 
> > > Right, I see. I'll take a careful look. How can we verify that all those
> > > operations really do not overflow?
> > 
> > Some of them are obvious, like subtracting 1 from the number of buckets (it
> can
> > never be < 4).
> > 
> > The ones that aren't so obvious are ones that calculate an element or bucket
> > index. But my intuiotion is that we'd have run out of memory before any of
> those
> > would overflow.
> 
> Any thoughts on the underlying deopt issue? This seems like either a flaw in
the
> way we write Hydrogen code today or a flaw in the INLINE_OPTIMIZED_FUNCTION
> list.

None of INLINE_OPTIMIZED_FUNCTIONS should ever deopt. (Since they do not learn,
there is no assumptions that might turn out to be false).
Fundamentally, when we write hydrogen for them we should ensure there is no
deopt points. 
Not sure how to assert that or even if it is worthwhile (you are just unlucky in
that your code does a lot of arithmetic)

[Dmitry Lomov (no reviews), 2014-12-08 10:54:05]

lgtm

[adamk, 2014-12-08 17:35:15]

The CQ bit was checked by adamk@chromium.org

[commit-bot: I haz the power, 2014-12-08 17:35:52]

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

[commit-bot: I haz the power, 2014-12-08 18:03:39]

Committed patchset #2 (id:20001)