Refactor to support multiple outputs in location summary

Refactor to support multiple outputs in location summary

R=srdjan@google.com

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

[Cutch, 2014-03-20 22:32:21]

Refactor LocationSummary to support multiple outputs.

I've left an ASSERT that the number of outputs is always 1 for now. Once full
support for multiple outputs is in place, I will remove the ASSERT.

[srdjan, 2014-03-20 23:04:23]

lgtm

https://codereview.chromium.org/207063002/diff/1/runtime/vm/intermediate_lang...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/207063002/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language.cc:2078: ASSERT((InputCount() == 0) ||
locs()->out(0).reg() == locs()->in(0).reg());
More parentheses pretty please.

https://codereview.chromium.org/207063002/diff/1/runtime/vm/locations.h
File runtime/vm/locations.h (right):

https://codereview.chromium.org/207063002/diff/1/runtime/vm/locations.h#newco...
runtime/vm/locations.h:400: intptr_t output_count = 1);
The count is kind of our of order. Instead of the optional argument you could
use  AddOutput (similar to AddTemp). Or maybe have a second constructor with 4
arguments, all counts together.

[Florian Schneider, 2014-03-21 10:47:06]

dbc:

https://codereview.chromium.org/207063002/diff/1/runtime/vm/locations.h
File runtime/vm/locations.h (right):

https://codereview.chromium.org/207063002/diff/1/runtime/vm/locations.h#newco...
runtime/vm/locations.h:479: ZoneGrowableArray<Location> input_locations_;
I wonder how often we actually want to add/remove operands from the location
summary. With growable arrays, we use at least 3*5 words for a (1, 1, 1)-operand
summary, which really contains 3 words of payload.

Have you considered using EmbeddedArray which is used in
intermediate_language.h?

[Cutch, 2014-03-21 14:36:45]

https://codereview.chromium.org/207063002/diff/1/runtime/vm/intermediate_lang...
File runtime/vm/intermediate_language.cc (right):

https://codereview.chromium.org/207063002/diff/1/runtime/vm/intermediate_lang...
runtime/vm/intermediate_language.cc:2078: ASSERT((InputCount() == 0) ||
locs()->out(0).reg() == locs()->in(0).reg());
On 2014/03/20 23:04:23, srdjan wrote:
> More parentheses pretty please.

Done.

https://codereview.chromium.org/207063002/diff/1/runtime/vm/locations.h
File runtime/vm/locations.h (right):

https://codereview.chromium.org/207063002/diff/1/runtime/vm/locations.h#newco...
runtime/vm/locations.h:400: intptr_t output_count = 1);
On 2014/03/20 23:04:23, srdjan wrote:
> The count is kind of our of order. Instead of the optional argument you could
> use  AddOutput (similar to AddTemp). Or maybe have a second constructor with 4
> arguments, all counts together.

Done.

https://codereview.chromium.org/207063002/diff/1/runtime/vm/locations.h#newco...
runtime/vm/locations.h:479: ZoneGrowableArray<Location> input_locations_;
On 2014/03/21 10:47:07, Florian Schneider wrote:
> I wonder how often we actually want to add/remove operands from the location
> summary. With growable arrays, we use at least 3*5 words for a (1, 1,
1)-operand
> summary, which really contains 3 words of payload.
> 
> Have you considered using EmbeddedArray which is used in
> intermediate_language.h?

Good point but I would prefer to do that change in a follow up CL.

[Cutch, 2014-03-21 14:40:41]

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