VM: Store edge counters in one per-function array.

VM: Store edge counters in one per-function array.

Reserve first element in the Function's ic_data_array to hold the edge
counter array.

Until now we had a one-element array per edge counter. This reduces memory
used by edge counters.

This CL allows to optimize code without having to have the unoptimized
code present.

Also, save space in Instruction by making place_id_ and lifetime_position_
a union. place_id_ is exclusively needed by Load/StoreOptimizer,
lifetime_position by the FlowGraphAllocator.

BUG=
R=asiva@google.com

Committed: https://github.com/dart-lang/sdk/commit/4a653d9e17c60f00d4600afdf27b2b57283eeaed

[Florian Schneider, 2015-09-17 11:04:23]

fschneider@google.com changed reviewers:
+ asiva@google.com, iposva@google.com

[Florian Schneider, 2015-09-17 11:04:23]

This should allow compiling optimized functions without having to generate
unoptimized code first.

[srdjan, 2015-09-23 19:24:43]

srdjan@google.com changed reviewers:
+ srdjan@google.com

[srdjan, 2015-09-23 19:24:44]

DBC

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/block_schedu...
File runtime/vm/block_scheduler.cc (right):

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/block_schedu...
runtime/vm/block_scheduler.cc:15: // Compute the edge count at the deopt id of a
TargetEntry or Goto.
s/deopt id/edge id/

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/block_schedu...
runtime/vm/block_scheduler.cc:62: const Array& ic_data_array = Array::Handle(
flow_graph()->zone(),

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

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:791:
Array::Handle(graph_compiler.edge_counters_array()));
zone,

[siva, 2015-09-26 00:16:58]

lgtm

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/block_schedu...
File runtime/vm/block_scheduler.cc (right):

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/block_schedu...
runtime/vm/block_scheduler.cc:16: static intptr_t ComputeEdgeCount(const Array&
edge_counters, intptr_t edge_id) {
This function does not seem to compute anything why is it called
ComputeEdgeCount?

[siva, 2015-09-26 00:18:32]

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler.cc (right):

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler.cc:202: LookupClass(Symbols::List()))),
I was wondering if it make sense to store these in the isolate instead of here
because it needs to be done just once for an isolate.

[Florian Schneider, 2015-09-28 09:19:43]

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/block_schedu...
File runtime/vm/block_scheduler.cc (right):

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/block_schedu...
runtime/vm/block_scheduler.cc:15: // Compute the edge count at the deopt id of a
TargetEntry or Goto.
On 2015/09/23 19:24:43, srdjan wrote:
> s/deopt id/edge id/

Done.

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/block_schedu...
runtime/vm/block_scheduler.cc:16: static intptr_t ComputeEdgeCount(const Array&
edge_counters, intptr_t edge_id) {
On 2015/09/26 00:16:58, siva wrote:
> This function does not seem to compute anything why is it called
> ComputeEdgeCount?

Done.

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/block_schedu...
runtime/vm/block_scheduler.cc:62: const Array& ic_data_array = Array::Handle(
On 2015/09/23 19:24:43, srdjan wrote:
> flow_graph()->zone(),

Done.

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

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:791:
Array::Handle(graph_compiler.edge_counters_array()));
On 2015/09/23 19:24:43, srdjan wrote:
> zone, 

Done.

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler.cc (right):

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler.cc:202: LookupClass(Symbols::List()))),
On 2015/09/26 00:18:32, siva wrote:
> I was wondering if it make sense to store these in the isolate instead of here
> because it needs to be done just once for an isolate.

Maybe. On the other hand they are only needed for compilation and cached here to
avoid creating too many ZoneHandles.

[Florian Schneider, 2015-09-28 11:28:09]

Committed patchset #4 (id:60001) manually as
4a653d9e17c60f00d4600afdf27b2b57283eeaed (presubmit successful).

[siva, 2015-09-28 17:01:29]

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/flow_graph_c...
File runtime/vm/flow_graph_compiler.cc (right):

https://codereview.chromium.org/1343383003/diff/20001/runtime/vm/flow_graph_c...
runtime/vm/flow_graph_compiler.cc:202: LookupClass(Symbols::List()))),
On 2015/09/28 09:19:43, Florian Schneider wrote:
> On 2015/09/26 00:18:32, siva wrote:
> > I was wondering if it make sense to store these in the isolate instead of
here
> > because it needs to be done just once for an isolate.
> 
> Maybe. On the other hand they are only needed for compilation and cached here
to
> avoid creating too many ZoneHandles.

Agreed it is done once per compilation but we have numerous compilations that
happen in an isolate and doing it once per isolate would save a bit of the per
compilation overhead cost.