[FFmpeg-devel] [PATCH] Move MLP's dot product to DSPContext

Michael Niedermayer michaelni
Fri May 15 21:43:13 CEST 2009


On Fri, May 15, 2009 at 04:33:12PM -0300, Ramiro Polla wrote:
> On Fri, May 15, 2009 at 3:43 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Fri, May 15, 2009 at 01:30:17PM -0300, Ramiro Polla wrote:
> >> On Fri, May 15, 2009 at 12:15 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> > On Fri, May 15, 2009 at 10:38:01AM -0300, Ramiro Polla wrote:
> >> >> On Fri, May 15, 2009 at 5:49 AM, Diego Biurrun <diego at biurrun.de> wrote:
> >> >> > On Thu, May 14, 2009 at 07:44:36PM -0300, Ramiro Polla wrote:
> >> >> >> #if ARCH_X86_64 || (ARCH_X86_32 && HAVE_7REGS)
> >> >> >>
> >> >> >>
> >> >> >> #if ARCH_X86_64
> >> >> >>
> >> >> >> #else /* ARCH_X86_32 && HAVE_7REGS */
> >> >> >>
> >> >> >> #endif
> >> >> >>
> >> >> >> #endif /* ARCH_X86_64 || (ARCH_X86_32 && HAVE_7REGS) */
> >> >> >
> >> >> > This had me confused for a moment, please comment the #endif.
> >> >>
> >> >> I used Loren's suggestion and added some comments to the endifs.
> >> >>
> >> >> I also fixed the indentation of the function declaration and
> >> >> simplified the constraints ifdef bit more. And also removed some
> >> >> unnecessarily exported symbols and renamed them.
> >> > [...]
> >> >> ? ? ? ? : [s]"r"(state),
> >> >> ? ? ? ? ? [c]"r"(coeff),
> >> >> ? ? ? ? ? [b]"r"(sample_buffer),
> >> >> ? ? ? ? ? [i]CONSTRAINT(blocksize),
> >> >> ? ? ? ? ? [mask ? ]CONSTRAINT((x86_reg)mask),
> >> >> ? ? ? ? ? [firjump]CONSTRAINT(firjump),
> >> >> ? ? ? ? ? [iirjump]CONSTRAINT(iirjump),
> >> >> ? ? ? ? ? [shift ?]COUNTER(filter_shift),
> >> >> ? ? ? ? ? [binc]"i"(sizeof(int32_t)* MAX_CHANNELS),
> >> >> ? ? ? ? ? [is ?]"i"(sizeof(int32_t)*(MAX_FIR_ORDER + MAX_BLOCKSIZE)),
> >> >> ? ? ? ? ? [ic ?]"i"(sizeof(int32_t)* MAX_FIR_ORDER)
> >> >
> >> > this would break gcc 2.95 (fate)
> >>
> >> It takes 11 operands on x86_32, so it doesn't compile on gcc-2.95
> >> anyways. Suggestions:
> >> - enclose it in HAVE_TEN_OPERANDS and leave the code with []s.
> >> - remove []s and hardcode binc, is and ic values.
> >>
> >> Is there a way to expand sizeof(int32_t)*foobar into the asm without
> >> using the "i" constraint?
> >
> > sizeof(int32_t) is 4
> 
> - Removed []s
> - Replaced "i"s by AVSTRINGIFYed defines.
> - Added const to [fi]irjump to avoid a warning.
[...]

>         "sub   $4       ,  %0         \n\t"
>         "mov  "RESULT32", (%0)        \n\t"
>         "mov  "RESULT32", (%2)        \n\t"
>         "add $"BINC"    ,  %2         \n\t"
>         "sub  "ACCUM"   ,"RESULT"     \n\t"
>         "mov  "RESULT32","IOFFS"(%0)  \n\t"
>         "incl              %3         \n\t"
>         "js 1b                        \n\t"
>         :
>         : /* 0*/"r"(state),

you are changing pure input operands, this leads to undefined behavior.
also mixed input+output count as 2 operands in relation to the 10 limit
in at least some gcc versions IIRC (so be carefull how you solve this)


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090515/c3bb010f/attachment.pgp>



More information about the ffmpeg-devel mailing list