AOT: Single target stub.

AOT: When a call goes polymorphic but has a single target, transition to a stub that does a cid range check.

Expand the range on a miss if all classes in the range have the same lookup result, otherwise transition to the stub that does a linear scan of ICData.

Because we do a depth-first sort of classes during AOT, if a call site sees only objects all inheriting the same function as the call target, the call site will be handled by this new stub.

Adjust LoadClassIdMayBeSmi on x64 to preserve the object.

R=fschneider@google.com

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

[rmacnak, 2016-08-29 21:49:58]

Description was changed from

==========
single target stub
==========

to

==========
AOT: When a call goes polymorphic but has a single target, transition to a stub
that does a cid range check.

Expand the range on a miss if all classes in the range have the same lookup
result, otherwise transition to the stub that does a linear scan of ICData.

Adjust LoadClassIdMayBeSmi on x64 to preserve the object.
==========

[rmacnak, 2016-08-29 21:50:15]

Description was changed from

==========
AOT: When a call goes polymorphic but has a single target, transition to a stub
that does a cid range check.

Expand the range on a miss if all classes in the range have the same lookup
result, otherwise transition to the stub that does a linear scan of ICData.

Adjust LoadClassIdMayBeSmi on x64 to preserve the object.
==========

to

==========
AOT: When a call goes polymorphic but has a single target, transition to a stub
that does a cid range check.

Expand the range on a miss if all classes in the range have the same lookup
result, otherwise transition to the stub that does a linear scan of ICData.

Adjust LoadClassIdMayBeSmi on x64 to preserve the object.
==========

[rmacnak, 2016-08-29 21:54:21]

Description was changed from

==========
AOT: When a call goes polymorphic but has a single target, transition to a stub
that does a cid range check.

Expand the range on a miss if all classes in the range have the same lookup
result, otherwise transition to the stub that does a linear scan of ICData.

Adjust LoadClassIdMayBeSmi on x64 to preserve the object.
==========

to

==========
AOT: When a call goes polymorphic but has a single target, transition to a stub
that does a cid range check.

Expand the range on a miss if all classes in the range have the same lookup
result, otherwise transition to the stub that does a linear scan of ICData.

Because we do a depth-first sort of classes during AOT, if a call site sees only
objects all inheriting the same function as the call target, the call site will
be handled by this new stub.

Adjust LoadClassIdMayBeSmi on x64 to preserve the object.
==========

[rmacnak, 2016-08-30 17:09:29]

rmacnak@google.com changed reviewers:
+ fschneider@google.com

[rmacnak, 2016-08-30 17:09:29]

layout benchmark: +5.1%
build benchmark: +8.27%

Handles things like Object.== Object.runtimeType Widget.key Element.slot
State.config State.config= IntegerImplementation.| IntegerImplementation.<<

Suspiciously Golem reports 6/4% improvements on FluidMotion/Tracer, even though
no call sites enter this new state.

[Florian Schneider, 2016-08-30 20:42:34]

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/code_genera...
File runtime/vm/code_generator.cc (right):

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/code_genera...
runtime/vm/code_generator.cc:1143: // Handle a miss of a megamorphic cache.
Comment: s/megamorphic/monomorphic/

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

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/object.h#ne...
runtime/vm/object.h:1857: }
Should we add a macro to define these accessors? e.g.

DEFINE_NON_POINTER_FIELD_ACCESSOR(intptr_t, upper_limit)

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/raw_object....
runtime/vm/raw_object.h:1494: intptr_t lower_limit_;
classid_t?

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/raw_object....
runtime/vm/raw_object.h:1495: intptr_t upper_limit_;
classid_t

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/stub_code_x...
File runtime/vm/stub_code_x64.cc (right):

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/stub_code_x...
runtime/vm/stub_code_x64.cc:2182: __ cmpq(RAX, R9);
Not sure if it's worth doing, but if upper_limit contains the difference between
the two (upper - lower), then you could do only one unsigned compare, like we do
for array bounds checks:

__ subq(RAX, R9);
__ cmpq(RAX, R10);
__ j(ABOVE, &miss);

[siva, 2016-08-30 23:54:53]

asiva@google.com changed reviewers:
+ asiva@google.com

[siva, 2016-08-30 23:54:53]

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/code_genera...
File runtime/vm/code_generator.cc (right):

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/code_genera...
runtime/vm/code_generator.cc:1036: ClassTable* table =
Isolate::Current()->class_table();
why not pass the isolate down to this function as it is available in the runtime
call instead of doing Isolate::Current() ?

[kasperl, 2016-08-31 04:56:06]

kasperl@google.com changed reviewers:
+ kasperl@google.com

[kasperl, 2016-08-31 04:56:07]

Cool stuff!

[rmacnak, 2016-08-31 16:42:57]

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/code_genera...
File runtime/vm/code_generator.cc (right):

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/code_genera...
runtime/vm/code_generator.cc:1036: ClassTable* table =
Isolate::Current()->class_table();
On 2016/08/30 23:54:53, siva wrote:
> why not pass the isolate down to this function as it is available in the
runtime
> call instead of doing Isolate::Current() ?

Done.

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/code_genera...
runtime/vm/code_generator.cc:1143: // Handle a miss of a megamorphic cache.
On 2016/08/30 20:42:34, Florian Schneider wrote:
> Comment: s/megamorphic/monomorphic/

Done.

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

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/object.h#ne...
runtime/vm/object.h:1857: }
On 2016/08/30 20:42:34, Florian Schneider wrote:
> Should we add a macro to define these accessors? e.g.
> 
> DEFINE_NON_POINTER_FIELD_ACCESSOR(intptr_t, upper_limit)

Done.

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/raw_object....
runtime/vm/raw_object.h:1494: intptr_t lower_limit_;
On 2016/08/30 20:42:34, Florian Schneider wrote:
> classid_t?

Done.

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/raw_object....
runtime/vm/raw_object.h:1495: intptr_t upper_limit_;
On 2016/08/30 20:42:34, Florian Schneider wrote:
> classid_t

Done.

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/stub_code_x...
File runtime/vm/stub_code_x64.cc (right):

https://codereview.chromium.org/2279563002/diff/150001/runtime/vm/stub_code_x...
runtime/vm/stub_code_x64.cc:2182: __ cmpq(RAX, R9);
On 2016/08/30 20:42:34, Florian Schneider wrote:
> Not sure if it's worth doing, but if upper_limit contains the difference
between
> the two (upper - lower), then you could do only one unsigned compare, like we
do
> for array bounds checks:
> 
> __ subq(RAX, R9);
> __ cmpq(RAX, R10);
> __ j(ABOVE, &miss);

I see -0.5% layout benchmark and +0.5% build benchmark, so I'll stick with base
and bound.

[Florian Schneider, 2016-08-31 16:50:39]

Lgtm

[rmacnak, 2016-08-31 18:12:26]

Description was changed from

==========
AOT: When a call goes polymorphic but has a single target, transition to a stub
that does a cid range check.

Expand the range on a miss if all classes in the range have the same lookup
result, otherwise transition to the stub that does a linear scan of ICData.

Because we do a depth-first sort of classes during AOT, if a call site sees only
objects all inheriting the same function as the call target, the call site will
be handled by this new stub.

Adjust LoadClassIdMayBeSmi on x64 to preserve the object.
==========

to

==========
AOT: When a call goes polymorphic but has a single target, transition to a stub
that does a cid range check.

Expand the range on a miss if all classes in the range have the same lookup
result, otherwise transition to the stub that does a linear scan of ICData.

Because we do a depth-first sort of classes during AOT, if a call site sees only
objects all inheriting the same function as the call target, the call site will
be handled by this new stub.

Adjust LoadClassIdMayBeSmi on x64 to preserve the object.

R=fschneider@google.com

Committed:
https://github.com/dart-lang/sdk/commit/ba94427eb13dff41b5eb5850219034aaf0016535
==========

[rmacnak, 2016-08-31 18:12:27]

Committed patchset #11 (id:190001) manually as
ba94427eb13dff41b5eb5850219034aaf0016535 (presubmit successful).