Make constant propagation to fold x == x and re-run type propagation for better range analysis.

Make constant propagation to fold x == x and re-run type propagation for better range analysis.

1. When comparing numbers, or strict-comparing objects, this can be folded into
true/false.  Since this often occurs after inlining and store-to-load forwarding,
constant propagation is repeated after these phases. The pattern looks like:

o.x = o.y;
if (o.x == o.y) { ... }

2. Load elimination may introduce new phis that may have smi-type. In order to
get range information for these phis, I added a second phase of type propagation after
load elimination.

R=kmillikin@google.com

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

[Kevin Millikin (Google), 2013-06-12 10:02:44]

LGTM.

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

https://codereview.chromium.org/16813002/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:5190: if (!instr->is_checked_strict_equal())
break;
I have two things to say about this.

1. It is really, really awesome.
2. You should not do it.

[Florian Schneider, 2013-06-12 10:11:20]

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

https://codereview.chromium.org/16813002/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:5190: if (!instr->is_checked_strict_equal())
break;
On 2013/06/12 10:02:44, kmillikin wrote:
> I have two things to say about this.
> 
> 1. It is really, really awesome.
> 2. You should not do it.

Undone. Using IsNumberCid instead of the switch.

[Florian Schneider, 2013-06-12 10:27:22]

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

[srdjan, 2013-06-12 16:37:51]

Not LGTM as there is a bug in the code:

Please add the following test:

foo(a) {
  return a == a;
}

main() {
  for (int i = 0; i < 10000; i++) {
    foo(3.0);
  }
  Expect.isFalse(foo(double.NAN));
}

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

https://codereview.chromium.org/16813002/diff/7002/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:5135: if (instr->left()->definition() ==
instr->right()->definition()) {
Yes, thanks to the spec that now says identical(NAN, NAN) => true

https://codereview.chromium.org/16813002/diff/7002/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:5189:
RawObject::IsNumberClassId(instr->receiver_class_id())) {
This is wrong for identical(NAN, NAN), which returns false. You can do this only
for int types.

[Florian Schneider, 2013-06-13 08:15:43]

I uploaded a fix for this: https://codereview.chromium.org/16853005/

Thanks for the catch.

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

https://codereview.chromium.org/16813002/diff/7002/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:5189:
RawObject::IsNumberClassId(instr->receiver_class_id())) {
On 2013/06/12 16:37:51, srdjan wrote:
> This is wrong for identical(NAN, NAN), which returns false. You can do this
only
> for int types.

Done.

[Lasse Reichstein Nielsen, 2013-06-13 08:21:25]

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

https://codereview.chromium.org/16813002/diff/7002/runtime/vm/flow_graph_opti...
runtime/vm/flow_graph_optimizer.cc:5189:
RawObject::IsNumberClassId(instr->receiver_class_id())) {
Is this correct? I.e. if
  var NAN = 0.0/0.0;
  print(identical(NAN,NAN));  
will it be able to print false?
I would have expected it to print true for identical(NAN,NAN), but false for
(NAN==NAN).