Optimize instanceof: if all results are true and tests can be done using class only replace instanc…

Optimize instanceof: if all results are true and tests can be done using class only replace instanceof with a Boolean constant.
Committed: https://code.google.com/p/dart/source/detail?r=16648

[regis, 2013-01-03 23:35:26]

https://codereview.chromium.org/11746024/diff/3001/runtime/vm/flow_graph_opti...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/11746024/diff/3001/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1258: if (!type.IsInstantiated()) return
Bool::null();
I suppose there is no need to check that !type.IsMalformed()?

https://codereview.chromium.org/11746024/diff/3001/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1268: false :
I do not think this is correct, because null is an instance of Object and of
dynamic.

[srdjan, 2013-01-04 00:10:55]

Please have another look

https://codereview.chromium.org/11746024/diff/3001/runtime/vm/flow_graph_opti...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/11746024/diff/3001/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1258: if (!type.IsInstantiated()) return
Bool::null();
On 2013/01/03 23:35:26, regis wrote:
> I suppose there is no need to check that !type.IsMalformed()?

Added it, just in case.

https://codereview.chromium.org/11746024/diff/3001/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1268: false :
On 2013/01/03 23:35:26, regis wrote:
> I do not think this is correct, because null is an instance of Object and of
> dynamic.

Fixed.

[regis, 2013-01-04 01:39:16]

LGTM, except the TODO(regis) :-)

https://codereview.chromium.org/11746024/diff/8001/runtime/vm/flow_graph_opti...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/11746024/diff/8001/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1267: // TODO(regis): Fix Class::TypeTest to
support NullClass.
I am pretty sure that Class::TypeTest is supporting NullClass properly. There is
a difference between "is subtype of", which applies to types and "is an instance
of", which applies to instances. Class::TypeTest() implements both "is a subtype
of" or "is more specific than" (according to the flag passed in), but not "is an
instance of".
Please note that null is an instance of Object (and of dynamic), but Null is not
a subtype of Object (or of any type).
This is why Instance::IsInstanceOf handles these special cases before calling
Class::IsSubTypeOf.

[Florian Schneider, 2013-01-04 11:35:41]

style dbc:

https://codereview.chromium.org/11746024/diff/8001/runtime/vm/flow_graph_opti...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/11746024/diff/8001/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1295: const Bool& negate =
Maybe just

bool negate =
  Bool::Cast(call->ArgumentAt(4)->value()->BoundConstant()).value()

and substitute negate.value() with just negate below.

[srdjan, 2013-01-04 21:10:25]

https://codereview.chromium.org/11746024/diff/8001/runtime/vm/flow_graph_opti...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/11746024/diff/8001/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1267: // TODO(regis): Fix Class::TypeTest to
support NullClass.
On 2013/01/04 01:39:16, regis wrote:
> I am pretty sure that Class::TypeTest is supporting NullClass properly. There
is
> a difference between "is subtype of", which applies to types and "is an
instance
> of", which applies to instances. Class::TypeTest() implements both "is a
subtype
> of" or "is more specific than" (according to the flag passed in), but not "is
an
> instance of".
> Please note that null is an instance of Object (and of dynamic), but Null is
not
> a subtype of Object (or of any type).
> This is why Instance::IsInstanceOf handles these special cases before calling
> Class::IsSubTypeOf.

Removed TODO. Discussed off-line changes to Class::TypeTest.

https://codereview.chromium.org/11746024/diff/8001/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:1295: const Bool& negate =
On 2013/01/04 11:35:41, Florian Schneider wrote:
> Maybe just
> 
> bool negate =
>   Bool::Cast(call->ArgumentAt(4)->value()->BoundConstant()).value()
> 
> and substitute negate.value() with just negate below.

Done.