Enable redundancy elimination for array loads.

Enable redundancy elimination for array loads.

This CL also adds a second round of CSE if necessary to make use
 of secondary effects for more optimization opportunities.
Committed: https://code.google.com/p/dart/source/detail?r=13859

[Florian Schneider, 2012-10-19 12:53:05]

This CL addresses the problem of missed optimization opportunities with indexed
loads on arrays.

[srdjan, 2012-10-19 14:45:50]

LGTM

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

https://codereview.chromium.org/11234002/diff/1/runtime/vm/compiler.cc#newcod...
runtime/vm/compiler.cc:219: // Do another round of CSE to take seconary effects
into account.
s/seconary/secondary/

https://codereview.chromium.org/11234002/diff/1/runtime/vm/compiler.cc#newcod...
runtime/vm/compiler.cc:219: // Do another round of CSE to take seconary effects
into account.
Can you give a brief example of secondary effects we are talking about here?

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

https://codereview.chromium.org/11234002/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2647: changed = OptimizeLoads(child,
&child_defs, avail_in) || changed;
Alternatives:

changed ||= OptimizeLoads...
changed = changed || OptimizedLoads ...

your call.

https://codereview.chromium.org/11234002/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2678: changed =
OptimizeRecursive(graph->graph_entry(), &map) || changed;
ditto

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

https://codereview.chromium.org/11234002/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.h:158: static bool Optimize(FlowGraph* graph);
Describe in comment what result true/false means

[Florian Schneider, 2012-10-22 10:31:55]

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

https://codereview.chromium.org/11234002/diff/1/runtime/vm/compiler.cc#newcod...
runtime/vm/compiler.cc:219: // Do another round of CSE to take seconary effects
into account.
On 2012/10/19 14:45:50, srdjan wrote:
> Can you give a brief example of secondary effects we are talking about here?

Done.

https://codereview.chromium.org/11234002/diff/1/runtime/vm/compiler.cc#newcod...
runtime/vm/compiler.cc:219: // Do another round of CSE to take seconary effects
into account.
On 2012/10/19 14:45:50, srdjan wrote:
> s/seconary/secondary/

Done.

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

https://codereview.chromium.org/11234002/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2647: changed = OptimizeLoads(child,
&child_defs, avail_in) || changed;
On 2012/10/19 14:45:50, srdjan wrote:
> Alternatives:
> 
> changed ||= OptimizeLoads...
> changed = changed || OptimizedLoads ...
> 
> your call.

I can't change the order in short-circuit || since OptimizedLoads won't be
evaluated when changed is already true.

https://codereview.chromium.org/11234002/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.cc:2678: changed =
OptimizeRecursive(graph->graph_entry(), &map) || changed;
On 2012/10/19 14:45:50, srdjan wrote:
> ditto

Done.

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

https://codereview.chromium.org/11234002/diff/1/runtime/vm/flow_graph_optimiz...
runtime/vm/flow_graph_optimizer.h:158: static bool Optimize(FlowGraph* graph);
On 2012/10/19 14:45:50, srdjan wrote:
> Describe in comment what result true/false means

Done.