[vm,aot] Improve performance of certain 'is' operations

[vm,aot] Improve performance of certain 'is' operations

AOT may infer the range of Cids to check in order to perform
'is' (instance-of) operation. It is possible that the range contains
the only Cid, so AOT can generate 1 comparison instead of 2.
Note that the optimization of 'x is T' where T is not extended nor
implemented does not fully cover this case, as T could be abstract with
a single subclass or implementor.

The notable real case for this optimization is 'x is double',
as double is an abstract class with single implementor _Double.

This CL also improves type propagation for InstanceOfInstr, and
enables printing of abstract types in IL printer.

R=rmacnak@google.com

Issue: https://github.com/dart-lang/sdk/issues/30480
Committed: https://github.com/dart-lang/sdk/commit/4f092b7e5fc501479c4475cfaa6a51887ab2261d

[alexmarkov, 2017-08-24 20:06:42]

alexmarkov@google.com changed reviewers:
+ asiva@google.com, rmacnak@google.com, vegorov@google.com

[rmacnak, 2017-08-24 21:21:01]

LGTM

[alexmarkov, 2017-08-24 21:27:52]

Description was changed from

==========
[vm,aot] Improve performance of certain 'is' operations

AOT may infer the range of Cids to check in order to perform
'is' (instance-of) operation. It is possible that the range contains
the only Cid, so AOT can generate 1 comparison instead of 2.
Note that the optimization of 'x is T' where T is not extended nor
implemented does not fully cover this case, as T could be abstract with
a single subclass or implementor.

The notable real case for this optimization is 'x is double',
as double is an abstract class with single implementor _Double.

This CL also improves type propagation for InstanceOfInstr, and
enables printing of abstract types in IL printer.

Issue: https://github.com/dart-lang/sdk/issues/30480
==========

to

==========
[vm,aot] Improve performance of certain 'is' operations

AOT may infer the range of Cids to check in order to perform
'is' (instance-of) operation. It is possible that the range contains
the only Cid, so AOT can generate 1 comparison instead of 2.
Note that the optimization of 'x is T' where T is not extended nor
implemented does not fully cover this case, as T could be abstract with
a single subclass or implementor.

The notable real case for this optimization is 'x is double',
as double is an abstract class with single implementor _Double.

This CL also improves type propagation for InstanceOfInstr, and
enables printing of abstract types in IL printer.

R=rmacnak@google.com

Issue: https://github.com/dart-lang/sdk/issues/30480
Committed:
https://github.com/dart-lang/sdk/commit/4f092b7e5fc501479c4475cfaa6a51887ab2261d
==========

[alexmarkov, 2017-08-24 21:27:58]

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

[Vyacheslav Egorov (Google), 2017-08-25 07:59:15]

lgtm

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

https://codereview.chromium.org/3003793002/diff/1/runtime/vm/aot_optimizer.cc...
runtime/vm/aot_optimizer.cc:1358: if (lower_limit == upper_limit) {
Does it make sense to do a similar optimization for `as` Checks?

There is a code below that handles as checks.

Maybe we should unify handling of is and as more? It does not make sense that
they are so duplicated right now.

https://codereview.chromium.org/3003793002/diff/1/runtime/vm/flow_graph_type_...
File runtime/vm/flow_graph_type_propagator.cc (right):

https://codereview.chromium.org/3003793002/diff/1/runtime/vm/flow_graph_type_...
runtime/vm/flow_graph_type_propagator.cc:329: InstanceOfInstr* instanceOf =
should be instance_of according to style guide

https://codereview.chromium.org/3003793002/diff/1/runtime/vm/flow_graph_type_...
runtime/vm/flow_graph_type_propagator.cc:331: bool is_simpleInstanceOf =
should be is_simple_instance_of

[alexmarkov, 2017-08-29 22:45:32]

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

https://codereview.chromium.org/3003793002/diff/1/runtime/vm/aot_optimizer.cc...
runtime/vm/aot_optimizer.cc:1358: if (lower_limit == upper_limit) {
On 2017/08/25 07:59:15, Vyacheslav Egorov (Google) wrote:
> Does it make sense to do a similar optimization for `as` Checks?
> 
> There is a code below that handles as checks.
> 
> Maybe we should unify handling of is and as more? It does not make sense that
> they are so duplicated right now.

I think you're right - 'as' and 'is' checks should be unified, preferably
lowered to the same kind of type test.

Without this unification, it doesn't look right to duplicate this optimization
for 'as' checks, because 'as' checks, unlike 'is' checks,  are implemented with
more heavyweight calls to helper functions and won't benefit much from this
particular optimization.

I'd like to consider unification of 'is' and 'as' type checks as a separate task
and approach it after implementing proper lowering pass.

Created issue: https://github.com/dart-lang/sdk/issues/30571.

https://codereview.chromium.org/3003793002/diff/1/runtime/vm/flow_graph_type_...
File runtime/vm/flow_graph_type_propagator.cc (right):

https://codereview.chromium.org/3003793002/diff/1/runtime/vm/flow_graph_type_...
runtime/vm/flow_graph_type_propagator.cc:329: InstanceOfInstr* instanceOf =
On 2017/08/25 07:59:15, Vyacheslav Egorov (Google) wrote:
> should be instance_of according to style guide

Done (in the next CL https://codereview.chromium.org/3010673002/).

https://codereview.chromium.org/3003793002/diff/1/runtime/vm/flow_graph_type_...
runtime/vm/flow_graph_type_propagator.cc:331: bool is_simpleInstanceOf =
On 2017/08/25 07:59:15, Vyacheslav Egorov (Google) wrote:
> should be is_simple_instance_of

Done.