Reland r38116: Improve receiver class check in polymorphic inlining.

Reland r38116: Improve receiver class check in polymorphic inlining.

Avoid reloading the class-id at the entry to last inlined variant at
inlined polymorphic call sites. For this I added a new check instruction
CheckClassIdInstr to deoptimize on integer-comparison.

In polymorphic CheckClass with small dense ranges of class-ids now generate
max. 2 branch instructions by doing a range check and computing a test mask
instead of a sequence of compare and branch instructions.

I fixed the crash bug which was due to illegal hoisting of CheckClassId and
two more assertion failures.

R=vegorov@google.com

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

[Florian Schneider, 2014-07-10 15:10:13]

https://codereview.chromium.org/381803005/diff/1/runtime/vm/intermediate_lang...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/381803005/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language.cc:2538: if (a.IsInfinity() || b.IsInfinity())
return overflow;
Handle infinity here in Add and Sub.

https://codereview.chromium.org/381803005/diff/1/runtime/vm/intermediate_lang...
File runtime/vm/intermediate_language.h (right):

https://codereview.chromium.org/381803005/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language.h:7981: virtual bool AllowsCSE() const { return
true; }
Correct flags and dependencies to allow CSE/LICM (same as CheckClassInstr).

https://codereview.chromium.org/381803005/diff/1/runtime/vm/intermediate_lang...
File runtime/vm/intermediate_language_ia32.cc (right):

https://codereview.chromium.org/381803005/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language_ia32.cc:5552:
ASSERT(unary_checks().GetReceiverClassIdAt(i) != kSmiCid);
Used unary_checks() instead of cids_ here.

[Vyacheslav Egorov (Google), 2014-07-10 15:27:53]

I fear this introduces excessive deopts in some cases.

See the question below

https://codereview.chromium.org/381803005/diff/1/runtime/vm/flow_graph_inline...
File runtime/vm/flow_graph_inliner.cc (right):

https://codereview.chromium.org/381803005/diff/1/runtime/vm/flow_graph_inline...
runtime/vm/flow_graph_inliner.cc:1446: new(isolate())
CheckClassIdInstr(new(isolate()) Value(load_cid),
I have a question about this code, now that you allowed CSE for CheckClassId,
LoadClassId can be CSEd as well, which means that CheckClassId can be hoisted
far away from this place. 

Previously we had a redefinition here preventing that (to avoid excessive
deopts), but now that is gone.

[Cutch, 2014-07-10 16:04:20]

https://codereview.chromium.org/381803005/diff/1/runtime/vm/intermediate_lang...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/381803005/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language.cc:2538: if (a.IsInfinity() || b.IsInfinity())
return overflow;
On 2014/07/10 15:10:13, Florian Schneider wrote:
> Handle infinity here in Add and Sub.

Why is this being added? I removed this from my 64-bit range CL.

[Vyacheslav Egorov (Google), 2014-07-10 16:34:56]

https://codereview.chromium.org/381803005/diff/1/runtime/vm/intermediate_lang...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/381803005/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language.cc:2538: if (a.IsInfinity() || b.IsInfinity())
return overflow;
On 2014/07/10 16:04:20, Cutch wrote:
> On 2014/07/10 15:10:13, Florian Schneider wrote:
> > Handle infinity here in Add and Sub.
> 
> Why is this being added? I removed this from my 64-bit range CL.

When trying to Add ranges we sometimes pass infinities here as a result of
canonicalizing symbolic boundary.

[Florian Schneider, 2014-07-11 11:38:30]

Added canonicalization for CheckClassId and simplified the instruction since it
always take a constant on the rhs.

https://codereview.chromium.org/381803005/diff/1/runtime/vm/flow_graph_inline...
File runtime/vm/flow_graph_inliner.cc (right):

https://codereview.chromium.org/381803005/diff/1/runtime/vm/flow_graph_inline...
runtime/vm/flow_graph_inliner.cc:1446: new(isolate())
CheckClassIdInstr(new(isolate()) Value(load_cid),
On 2014/07/10 15:27:53, Vyacheslav Egorov (Google wrote:
> I have a question about this code, now that you allowed CSE for CheckClassId,
> LoadClassId can be CSEd as well, which means that CheckClassId can be hoisted
> far away from this place. 
> 
> Previously we had a redefinition here preventing that (to avoid excessive
> deopts), but now that is gone.

I added a Redefinition for the LoadCid instruction.

[Vyacheslav Egorov (Google), 2014-07-11 11:50:24]

lgtm

https://codereview.chromium.org/381803005/diff/20001/runtime/vm/intermediate_...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/381803005/diff/20001/runtime/vm/intermediate_...
runtime/vm/intermediate_language.cc:2014: if (value()->BindsToConstant()) {
does BindsToConstant() see through to OriginalDefinition?

note: constant folding of LoadClassId instruction does not take externalization
into account. we need to fix this.

[Florian Schneider, 2014-07-11 12:08:50]

Committed patchset #2 manually as r38154 (presubmit successful).

[Florian Schneider, 2014-07-14 12:13:46]

https://codereview.chromium.org/381803005/diff/20001/runtime/vm/intermediate_...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/381803005/diff/20001/runtime/vm/intermediate_...
runtime/vm/intermediate_language.cc:2014: if (value()->BindsToConstant()) {
On 2014/07/11 11:50:24, Vyacheslav Egorov (Google wrote:
> does BindsToConstant() see through to OriginalDefinition?
>

No, we run Canonicalize after removing Redefinitions though.
 
> note: constant folding of LoadClassId instruction does not take
externalization
> into account. we need to fix this.

Yes.