MIPS: Ensure proper alignment of LazyInstance objects.

MIPS: Ensure proper alignment of LazyInstance objects.

The template system converts the actual struct type to an array of chars.
Make sure the alignment is kept by the compiler.

This fixes a lot of serialization-related HW tests, for example cctest test-serialize/Serialize.

BUG=
TEST=cctest test-serialize

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

[Philippe, 2012-03-16 12:24:30]

http://codereview.chromium.org/9702114/diff/1/src/lazy-instance.h
File src/lazy-instance.h (right):

http://codereview.chromium.org/9702114/diff/1/src/lazy-instance.h#newcode188
src/lazy-instance.h:188: // Note that the previous field, OnceType, is an
AtomicWord which guarantees
We noticed this issue and thought that we could temporarily rely on the
AtomicWord field (once_) to guarantee the correct alignment. Does this issue
apply to architectures other than MIPS? Should we always use the alignof
attribute (with GCC)? You could probably update the comment here.

Thanks for fixing this.

[danno, 2012-03-16 12:40:08]

http://codereview.chromium.org/9702114/diff/1/src/lazy-instance.h
File src/lazy-instance.h (right):

http://codereview.chromium.org/9702114/diff/1/src/lazy-instance.h#newcode107
src/lazy-instance.h:107: #if V8_HOST_ARCH_MIPS
I'd prefer avoiding the platform-specific define. Is there a more general test
about the target architecture that can determine if this is needed? Is it
harmful to have this always present for GCC-based builds?

http://codereview.chromium.org/9702114/diff/1/src/lazy-instance.h#newcode188
src/lazy-instance.h:188: // Note that the previous field, OnceType, is an
AtomicWord which guarantees
On 2012/03/16 12:24:30, Philippe wrote:
> We noticed this issue and thought that we could temporarily rely on the
> AtomicWord field (once_) to guarantee the correct alignment. Does this issue
> apply to architectures other than MIPS? Should we always use the alignof
> attribute (with GCC)? You could probably update the comment here.
> 
> Thanks for fixing this.

Done.

[kalmard, 2012-03-16 12:56:30]

Thanks for the quick replies!

In case any of the questions was directed at me: I believe using alignof should
not cause harm on any architecture.

And some clarification: the once_ workaround works for 4 byte alignment issues.
However MIPS uses 8 byte alignment in some cases, which is why this fix is
needed.

[Philippe, 2012-03-16 13:12:02]

On 2012/03/16 12:56:30, kalmard wrote:
> Thanks for the quick replies!
> 
> In case any of the questions was directed at me: I believe using alignof
should
> not cause harm on any architecture.
> 
> And some clarification: the once_ workaround works for 4 byte alignment
issues.
> However MIPS uses 8 byte alignment in some cases, which is why this fix is
> needed.

Great, so maybe you can replace your MIPS ifdef with a GCC ifdef and modify the
comment to make it say something like "TODO: Fix alignment issue on MIPS with
non-GCC compilers."?

[kalmard, 2012-03-17 10:32:47]

On 2012/03/16 13:12:02, Philippe wrote:
> Great, so maybe you can replace your MIPS ifdef with a GCC ifdef and modify
the
> comment to make it say something like "TODO: Fix alignment issue on MIPS with
> non-GCC compilers."?

Done. I also kept part of the original comment, since it's still valid when not
compiling with gcc.

[Philippe, 2012-03-19 09:03:28]

On 2012/03/17 10:32:47, kalmard wrote:
> On 2012/03/16 13:12:02, Philippe wrote:
> > Great, so maybe you can replace your MIPS ifdef with a GCC ifdef and modify
> the
> > comment to make it say something like "TODO: Fix alignment issue on MIPS
with
> > non-GCC compilers."?
> 
> Done. I also kept part of the original comment, since it's still valid when
not
> compiling with gcc.

LGTM.

[fschneider, 2012-03-20 11:02:25]

lgtm

[Paul Lind, 2012-03-21 07:06:24]

Unfortunately, this is broken on mac (gcc 4.2.1, on osx 10.6.8). 

The '__attribute__((aligned(__alignof__(x))))' generates error: requested
alignment is not a constant.

This is also broken for gcc 4.2.x on Linux, though most of us use a newer gcc
(it works in 4.3.x and later). Sorry, I don't have a suggestion for a fix yet.

[kalmard, 2012-03-22 09:30:25]

Updated issue based on Paul's and Gergely's recommendation.

[Philippe, 2012-03-22 09:34:02]

On 2012/03/22 09:30:25, kalmard wrote:
> Updated issue based on Paul's and Gergely's recommendation.

LGTM.

[kalmard, 2012-03-31 11:12:41]

rebased on r11201

[Philippe, 2012-04-02 07:35:54]

On 2012/03/31 11:12:41, kalmard wrote:
> rebased on r11201

LGTM.

[kalmard, 2012-04-05 08:45:24]

rebased on r11230