Allocation space decisions are precisely made in hydrogen.

Allocation space decisions are precisely made in hydrogen.

BUG=
R=mstarzinger@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=15970

[Michael Starzinger, 2013-07-30 12:50:41]

https://codereview.chromium.org/21089006/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/21089006/diff/1/src/hydrogen-instructions.h#n...
src/hydrogen-instructions.h:5281: static Flags SetAllocationFlag(Flags flags,
Flags allocation_flag) {
As discussed offline: I don't like this helper very much, it just has minor
advantages over manipulating the flags field by hand. IMHO the DefaultFlags()
helper could instead take a PretenureFlag and already return the correct flags.
This works everywhere expect for FastLiteral, for which we have other plans
anyways.

https://codereview.chromium.org/21089006/diff/1/src/hydrogen-instructions.h#n...
src/hydrogen-instructions.h:5347: static const int kAllocationFlagsMask = 0x7;
Better compute this using the above three enum values, otherwise it might become
out of sync.

[Hannes Payer (out of office), 2013-07-30 15:23:04]

https://codereview.chromium.org/21089006/diff/1/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/21089006/diff/1/src/hydrogen-instructions.h#n...
src/hydrogen-instructions.h:5281: static Flags SetAllocationFlag(Flags flags,
Flags allocation_flag) {
On 2013/07/30 12:50:41, Michael Starzinger wrote:
> As discussed offline: I don't like this helper very much, it just has minor
> advantages over manipulating the flags field by hand. IMHO the DefaultFlags()
> helper could instead take a PretenureFlag and already return the correct
flags.
> This works everywhere expect for FastLiteral, for which we have other plans
> anyways.

I took that idea even further and made the HAllocate flags private and
internalized the allocation decisions.

https://codereview.chromium.org/21089006/diff/1/src/hydrogen-instructions.h#n...
src/hydrogen-instructions.h:5347: static const int kAllocationFlagsMask = 0x7;
On 2013/07/30 12:50:41, Michael Starzinger wrote:
> Better compute this using the above three enum values, otherwise it might
become
> out of sync.

Removed, not needed anymore

[Michael Starzinger, 2013-07-30 16:51:16]

LGTM. Just one suggestion, but feel free to ignore it.

https://codereview.chromium.org/21089006/diff/7001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/21089006/diff/7001/src/hydrogen-instructions....
src/hydrogen-instructions.h:5313: void SetPrefillWithFiller() {
suggestion: Maybe s/SetPrefillWithFiller/MakePrefillWithFiller/ here, but I am
also fine with leaving it as it is, up to you.

https://codereview.chromium.org/21089006/diff/7001/src/hydrogen-instructions....
src/hydrogen-instructions.h:5317: void SetDoubleAligned() {
Likewise.

[Hannes Payer (out of office), 2013-07-31 07:02:41]

https://codereview.chromium.org/21089006/diff/7001/src/hydrogen-instructions.h
File src/hydrogen-instructions.h (right):

https://codereview.chromium.org/21089006/diff/7001/src/hydrogen-instructions....
src/hydrogen-instructions.h:5313: void SetPrefillWithFiller() {
On 2013/07/30 16:51:16, Michael Starzinger wrote:
> suggestion: Maybe s/SetPrefillWithFiller/MakePrefillWithFiller/ here, but I am
> also fine with leaving it as it is, up to you.

Done.

https://codereview.chromium.org/21089006/diff/7001/src/hydrogen-instructions....
src/hydrogen-instructions.h:5317: void SetDoubleAligned() {
On 2013/07/30 16:51:16, Michael Starzinger wrote:
> Likewise.

Done.

[Hannes Payer (out of office), 2013-07-31 07:03:24]

Committed patchset #4 manually as r15970 (presubmit successful).