Merge RawICData::range_feedback_ field into RawICData::state_bits_.

Merge RawICData::range_feedback_ field into RawICData::state_bits_.

We were using a lot of bits in state_bits for recording deoptimization reasons, never asked about presence of those bits.

Ultimately optimizer cares only about 3 different deopts, so most all of those bits were wasted.

R=srdjan@google.com, zra@google.com
BUG=

Committed: https://code.google.com/p/dart/source/detail?r=42388

[Vyacheslav Egorov (Google), 2014-12-15 20:44:17]

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

[Vyacheslav Egorov (Google), 2014-12-15 20:44:18]

Srdjan & Zach, here is a follow up CL that removes range_feedback_ field
overhead on 64-bit platforms. 

I considered to drop most of the deopt reasons entirely, but decided to keep
them for now, though I am not sure if we really need them for anything other
than --trace-deoptimization.

[zra, 2014-12-15 20:57:38]

lgtm

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

https://codereview.chromium.org/807593002/diff/1/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:11443: if (reason <= kLastRecordedDeoptReason) {
Why is this not an assert?

https://codereview.chromium.org/807593002/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/807593002/diff/1/runtime/vm/object.h#newcode3550
runtime/vm/object.h:3550: // Note: only deopts with reasons below kUnknown in
this list are recorded in
"below" could be a little confusing. I first read it as meaning "after Unknown
in the following list". Maybe change to "before Unknown in following list," or
"with values less than kUnknown."

[Vyacheslav Egorov (Google), 2014-12-15 21:02:21]

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

https://codereview.chromium.org/807593002/diff/1/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:11443: if (reason <= kLastRecordedDeoptReason) {
On 2014/12/15 20:57:38, zra wrote:
> Why is this not an assert?

Caller (deoptimizer) can try to add deopt reason which we don't care about -
this is fine, we just drop it to the floor.

On the other hand if optimizer is checking (via HasDeoptReason) for a deopt
reason, which we don't record, then this is a bug. Because reason has to be
recordable for HasDeoptReason to make sense.

That's why this is guarded with an if and the other one is an assert.

Alternatively I could move this *if* to the caller (deoptimizer), but that felt
like a wrong place.

WDYT?

[zra, 2014-12-15 21:04:58]

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

https://codereview.chromium.org/807593002/diff/1/runtime/vm/object.cc#newcode...
runtime/vm/object.cc:11443: if (reason <= kLastRecordedDeoptReason) {
On 2014/12/15 21:02:21, Vyacheslav Egorov (Google) wrote:
> On 2014/12/15 20:57:38, zra wrote:
> > Why is this not an assert?
> 
> Caller (deoptimizer) can try to add deopt reason which we don't care about -
> this is fine, we just drop it to the floor.
> 
> On the other hand if optimizer is checking (via HasDeoptReason) for a deopt
> reason, which we don't record, then this is a bug. Because reason has to be
> recordable for HasDeoptReason to make sense.
> 
> That's why this is guarded with an if and the other one is an assert.
> 
> Alternatively I could move this *if* to the caller (deoptimizer), but that
felt
> like a wrong place.
> 
> WDYT? 

Makes sense. I think it's fine as-is. Thanks!

[srdjan, 2014-12-16 00:26:07]

lgtm

[Vyacheslav Egorov (Google), 2014-12-16 13:12:47]

Committed patchset #1 (id:1) manually as 42388 (presubmit successful).