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

Ramiro Polla ramiro.polla
Sat May 23 02:24:26 CEST 2009


On Tue, May 19, 2009 at 11:36 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Fri, May 15, 2009 at 09:41:30PM -0300, Ramiro Polla wrote:
>> On Fri, May 15, 2009 at 4:43 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> > 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)
>>
>> #if HAVE_TEN_OPERANDS ?
>
> hmm, well ok
> and patch ok, less macro would be better though ...

Applied with the #endif comment as /* !HAVE_X86_64 */

Ramiro Polla



More information about the ffmpeg-devel mailing list