[PATCH] test: only test for double alignment on 64 bit. (#36986)

[Mark Kettenis]

> Date: Thu, 19 May 2011 14:54:28 +1000
> From: Peter Hutterer <peter.hutterer at who-t.net>
> 
> X.Org Bug 36986 <http://bugs.freedesktop.org/show_bug.cgi?id=36986>
> 
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> 
> On Wed, May 18, 2011 at 12:45:05AM -0700, Keith Packard wrote:
> > On Wed, 18 May 2011 13:30:44 +1000, Peter Hutterer
> > <peter.hutterer at who-t.net> wrote:
> > 
> > > +    const int sz_dbl = sizeof(double);
> > >      /* force alignment with double */
> > > -    union align_u { ValuatorClassRec valc; double d; } *align;
> > > +    union align_u {
> > > +        ValuatorClassRec valc;
> > > +        double d[(sizeof(ValuatorClassRec) + sz_dbl - 1)/sz_dbl];
> > > +    } *align;
> > 
> > This doesn't make sense to me. The requirement for the union is that you
> > be able to allocate an array of them and store into each element:
> > 
> >         union align_u   foo[12];
> > 
> >         foo[0].d = 0.0;
> >         foo[1].d = 1.0;
> > 
> > This should require double alignment for the entire union, even though
> > 'd' is far smaller than 'valc'.
> > 
> > Of course, on a 32-bit x86 machine, doubles can be stored without
> > penalty on 4-byte boundaries, so the union is only aligned to 52 bytes.
> > 
> > Can you explain why this alignment isn't acceptable in this context?
> 
> because I naïvely thought that we needed double alignment on 32 bit too and
> the test was written that way. Oh well, how about this patch instead then.
> it just disables the alignment check on 32 bit.

The idea that you only need "double" alignment on 64-bit platforms is
just as naive as the idea that you need "double" alignment on all
platforms.  Or perhaps even more naive.  The real answer is that
"bitness" has very little to do with this.  Most, if not all, 32-bit
RISC platforms need proper alignment for example.  Of course the
compiler will take care of this.  I believe that even the ones where
the OS kernel fixes up unaligned loads and stores of integers still
require properly aligned floating point numbers.

I believe that i386 pretty much is the odd one out here.  So perhaps
replacing that #ifdef _LP64 with #ifdef __i386__ would be a better
idea.

> diff --git a/test/input.c b/test/input.c
> index ac37d67..3c310da 100644
> --- a/test/input.c
> +++ b/test/input.c
> @@ -1223,8 +1223,11 @@ static void dix_valuator_alloc(void)
>  
>          assert(v);
>          assert(v->numAxes == num_axes);
> +#ifdef _LP64
> +        /* must be double-aligned on 64 bit */
>          assert(((void*)v->axisVal - (void*)v) % sizeof(double) == 0);
>          assert(((void*)v->axes - (void*)v) % sizeof(double) == 0);
> +#endif
>          num_axes ++;
>      }
>  
> -- 
> 1.7.4.4
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 
>