Try aligning unboxed double array backing store in allocation or scavenge promotion.

Try aligning unboxed double array backing store in allocation or scavenge promotion.

This CL does not align them during compaction or mark-sweep promotion because we are not using specialized evacuation visitors. 

R=erik.corry@gmail.com

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

[Erik Corry, 2012-04-16 14:35:19]

LGTM

https://chromiumcodereview.appspot.com/10001012/diff/1/src/heap.cc
File src/heap.cc (right):

https://chromiumcodereview.appspot.com/10001012/diff/1/src/heap.cc#newcode1490
src/heap.cc:1490: int size)) {
This assumes that there is an even number of header fields ahead of the double
fields.  Add static assertion?

https://chromiumcodereview.appspot.com/10001012/diff/1/src/heap.cc#newcode1576
src/heap.cc:1576: enum AlignmentRequirement { POINTER_ALIGNED, DOUBLE_ALIGNED };
It seems to me that if you use the actual alignment requirements here instead of
0 and 1 in the enum then you will not get two different copies of the function
created by the template engine on 64 bit architectures.

https://chromiumcodereview.appspot.com/10001012/diff/1/src/heap.cc#newcode1642
src/heap.cc:1642: if (alignment == DOUBLE_ALIGNED) {
You can replace this with
if (kPointerSize != alignment)
and get it folded away on 64 bit without having an ifdef.  Also several other
places.

https://chromiumcodereview.appspot.com/10001012/diff/1/src/heap.cc#newcode1734
src/heap.cc:1734: slot,
Ironically enough, the alignment is wrong here.

https://chromiumcodereview.appspot.com/10001012/diff/1/src/ia32/codegen-ia32.cc
File src/ia32/codegen-ia32.cc (right):

https://chromiumcodereview.appspot.com/10001012/diff/1/src/ia32/codegen-ia32....
src/ia32/codegen-ia32.cc:400: __ lea(esi, Operand(edi, times_4,
FixedDoubleArray::kHeaderSize + kPointerSize));
Lint?

https://chromiumcodereview.appspot.com/10001012/diff/1/src/ia32/codegen-ia32....
src/ia32/codegen-ia32.cc:404: __ test(eax, Immediate(0x6));
It seems strange to test the 2 bit here when we know it is always zero.

https://chromiumcodereview.appspot.com/10001012/diff/1/src/ia32/codegen-ia32....
src/ia32/codegen-ia32.cc:409: __ j(zero, &aligned_done, Label::kNear);
Why is this branch conditional?

https://chromiumcodereview.appspot.com/10001012/diff/1/src/v8globals.h
File src/v8globals.h (right):

https://chromiumcodereview.appspot.com/10001012/diff/1/src/v8globals.h#newcode51
src/v8globals.h:51: #if V8_HOST_ARCH_64_BIT
Instead of the ifdef you could just set kDoubleAlignment to 8.

[Vyacheslav Egorov (Chromium), 2012-04-30 14:39:11]

landed (mailing my draft comments)

http://codereview.chromium.org/10001012/diff/1/src/heap.cc
File src/heap.cc (right):

http://codereview.chromium.org/10001012/diff/1/src/heap.cc#newcode1490
src/heap.cc:1490: int size)) {
On 2012/04/16 14:35:19, Erik Corry wrote:
> This assumes that there is an even number of header fields ahead of the double
> fields.  Add static assertion?

Done.

http://codereview.chromium.org/10001012/diff/1/src/heap.cc#newcode1642
src/heap.cc:1642: if (alignment == DOUBLE_ALIGNED) {
On 2012/04/16 14:35:19, Erik Corry wrote:
> You can replace this with
> if (kPointerSize != alignment)
> and get it folded away on 64 bit without having an ifdef.  Also several other
> places.

Done.

http://codereview.chromium.org/10001012/diff/1/src/heap.cc#newcode1642
src/heap.cc:1642: if (alignment == DOUBLE_ALIGNED) {
On 2012/04/16 14:35:19, Erik Corry wrote:
> You can replace this with
> if (kPointerSize != alignment)
> and get it folded away on 64 bit without having an ifdef.  Also several other
> places.

Done.

http://codereview.chromium.org/10001012/diff/1/src/ia32/codegen-ia32.cc
File src/ia32/codegen-ia32.cc (right):

http://codereview.chromium.org/10001012/diff/1/src/ia32/codegen-ia32.cc#newco...
src/ia32/codegen-ia32.cc:400: __ lea(esi, Operand(edi, times_4,
FixedDoubleArray::kHeaderSize + kPointerSize));
On 2012/04/16 14:35:19, Erik Corry wrote:
> Lint?

Done.

http://codereview.chromium.org/10001012/diff/1/src/ia32/codegen-ia32.cc#newco...
src/ia32/codegen-ia32.cc:409: __ j(zero, &aligned_done, Label::kNear);
On 2012/04/16 14:35:19, Erik Corry wrote:
> Why is this branch conditional?

Done.