[FFmpeg-devel] [PATCH] Move DECLARE_ALIGNED_{8, 16} macros to mem.h

[Pavel Pavlov]

> On Mon, Mar 08, 2010 at 06:21:29PM -0500, Pavel Pavlov wrote:
> > > Subject: Re: [FFmpeg-devel] [PATCH] Move DECLARE_ALIGNED_{8, 16}
> > > macros to mem.h
> > >
> > > Pavel Pavlov <pavel at summit-tech.ca> writes:
> > >
> > > >> > On Sat, Mar 06, 2010 at 01:27:56PM +0000, M?ns Rullg?rd wrote:
> > > >> >> Michael Niedermayer <michaelni at gmx.at> writes:
> > > >> >>
> > > >> >> > On Sat, Mar 06, 2010 at 02:50:30AM +0000, Mans Rullgard
> wrote:
> > > >> >> >> These macros naturally belong next to the generic
> > > DECLARE_ALIGNED
> > > >> >> >> macro.
> > > >> >> >> ---
> > > >> >> >>  libavcodec/dsputil.h |    3 ---
> > > >> >> >>  libavutil/mem.h      |    2 ++
> > > >> >> >>  2 files changed, 2 insertions(+), 3 deletions(-)
> > > >> >> >
> > > >> >> > ok if they arent redundant but they look a little redundant
> > > >> >> > relative to just using DECLARE_ALIGNED directly
> > > >> >>
> > > >> >> I always wondered what they were for.  OK to do a global
> > > >> >> search
> > > and
> > > >> >> replace?
> > > >> >
> > > >> > ok
> > > >>
> > > >> Done
> > > >
> > > > There is also ATTR_ALIGN left out in a couple of files
> > > > (dsputil_vis.c, fdct_mmx.c, idct_mmx.c). Should also be replaced
> > > > with DECLARE_ALIGNED
> > >
> > > Ugh, I've missed those.  Patches welcome.
> > >
> >
> > By the way, I remember sending in patches for this macro like a year
> ago. I've been building ffmpeg with these changes for a while because
> my compiler needs different compiler directives.
> > There is a couple of tricky places that might need some code review,
> like these:
> > From
> > static struct
> > {
> >  const int32_t fdct_r_row_sse2[4] ATTR_ALIGN[16]; } fdct_r_row_sse2
> > ATTR_ALIGN[16] =
> >
> > It became
> > DECLARE_ALIGNED(16, static struct
> > {
> >  DECLARE_ALIGNED(16, const int32_t, fdct_r_row_sse2[4]); },
> > fdct_r_row_sse2) =
> >
> >
> > But to me it seems that the original code had to be modified, since
> these ATTR_ALIGN are duplicates: if inner fdct_r_row_sse2 is aligned on
> 16, then the outer fdct_r_row_sse2 is also aligned on 16 and opposite
> is also true.
> 
> if a member of a struct requires alignment then the compiler must align
> the struct, if it does not without an explicit align of the struct it
> is buggy


Patchek traling whitespace fix. By the way, what to do with that array of structs and why was it originally changed to be that way?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: DECLARE_ALIGNED.patch
Type: application/octet-stream
Size: 8580 bytes
Desc: DECLARE_ALIGNED.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100309/b0e6d8b0/attachment.obj>