Convert EqualityCompare to StrictCompare if reciever is either null or guaranteed to have default e…

Convert EqualityCompare to StrictCompare if reciever is either null or guaranteed to have default equality operator.

R=srdjan@google.com
BUG=

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

[srdjan, 2013-04-18 16:54:49]

LGTM with comments

https://codereview.chromium.org/13878017/diff/1/runtime/vm/flow_graph_optimiz...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/13878017/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2190: bool
FlowGraphOptimizer::CanStrictifyEqualityCompare(
Can this be a const function?

https://codereview.chromium.org/13878017/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2219: 0));
Don't you need to look up the super chain if any (but not Object) class
implements equality operator?

https://codereview.chromium.org/13878017/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2231: T current_instruction) {
Can this be a const function?

https://codereview.chromium.org/13878017/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2231: T current_instruction) {
I assume that it is templated bcause current_instruction could be either
Definition or ControlInstruction?

[srdjan, 2013-04-18 17:28:59]

https://codereview.chromium.org/13878017/diff/1/runtime/vm/flow_graph_optimiz...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/13878017/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2219: 0));
On 2013/04/18 16:54:49, srdjan wrote:
> Don't you need to look up the super chain if any (but not Object) class
> implements equality operator?

Correction: ResolveDynamicForReceiverClass goes up the chain. I wonder how does
it work since there is always going to be at least the Object's equality
operator function as result.

[Vyacheslav Egorov (Google), 2013-04-18 19:48:36]

Thanks for the review. 

Landing.

https://codereview.chromium.org/13878017/diff/1/runtime/vm/flow_graph_optimiz...
File runtime/vm/flow_graph_optimizer.cc (right):

https://codereview.chromium.org/13878017/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2190: bool
FlowGraphOptimizer::CanStrictifyEqualityCompare(
On 2013/04/18 16:54:49, srdjan wrote:
> Can this be a const function?

Made it static.

https://codereview.chromium.org/13878017/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2219: 0));
On 2013/04/18 17:28:59, srdjan wrote:
> On 2013/04/18 16:54:49, srdjan wrote:
> > Don't you need to look up the super chain if any (but not Object) class
> > implements equality operator?
> 
> Correction: ResolveDynamicForReceiverClass goes up the chain. I wonder how
does
> it work since there is always going to be at least the Object's equality
> operator function as result.

Object has kInstanceCid cid that is how it works. Though it is quite confusing.

https://codereview.chromium.org/13878017/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2231: T current_instruction) {
On 2013/04/18 16:54:49, srdjan wrote:
> Can this be a const function?

Made it const.

https://codereview.chromium.org/13878017/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2231: T current_instruction) {
On 2013/04/18 16:54:49, srdjan wrote:
> I assume that it is templated bcause current_instruction could be either
> Definition or ControlInstruction?

Yes, for the same reason the HandleEqualityCompare is templated.

I think we should really refactor our handling of comparisons: make them always
hosted either inside of the BranchInstr or IfThenElseInstr.

I'll keep that in the list of things to do.

[Vyacheslav Egorov (Google), 2013-04-18 19:52:36]

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