[Ffmpeg-devel] Re: [PATCH] fix snow x86 SIMD

[Michael Niedermayer]

Hi

On Thu, Aug 10, 2006 at 02:42:11PM +0200, Martin von Gagern wrote:
> Michael Niedermayer wrote:
> > does the code compile with gcc 3.4 & 2.95 on (32bit) x86 and do regression
> > tests pass on these?
> > if no then the patch is rejected, if yes ill review it more carefull
> 
> 2.95 kills me - "more than 10 operands in `asm'". :-(
> So dies my hope of having gcc take care of allocating registers.
> 
> Back one step. At least the snowdsp_mmx.c code does not rely on REG_b
> actually being ebx as far as I can tell. So I changed this to ebp again,
> renaming the thing from REG_b to REG_bxp (a mix of bx and bp).

maybe you could just make ebx -> "r"?

> 
> This now compiles using gcc 2.95.3, 3.4.6 or 4.1.1, and passes
> regression tests for each of these with the default configuration.
> No surprise there, as without PIC the code stays the same after
> preprocessing.
> 
> The code currently fails regression tests when configured with
>   --extra-cflags="-DPIC -fPIC -fomit-frame-pointer"
>   --disable-static --enable-shared
> The program dies with SIGSEGV at the position corresponding to line 776:
>              "paddd (%%"REG_D"), %%xmm0      \n\t"
> After some debugging I found out that REG_D % 16 == 8 at this location,
> whereas IA32 Handbook requires it to be 16 byte aligned, causing a
> general protection exception otherwise, which explains an SIGSEGV imho.
> 
> I cannot see how my change could be the cause of this problem.
> If things accidentially were only 8 byte aligned, you'd have a 50%
> chance of tests still succeeding. So perhaps it was just that using PIC
> and the likes somehow shifted memory alignment and I now experience this
> bug because of it.
> 
> Would you go through your code and see if it correctly ensures 16 byte
> alignment for the address assembled in REG_D? Or maybe add a few
> assertions so I can test where things are no longer aligned that should
> have been? I would very much appreciate that.

if the code where not guranteeing alignment it likely would have failed
on someone elses system ...
* check that av_malloc() provides aligned memory
* check that gcc assembled the code correctly, ive seen cases where gcc
  silently violated some constrains
* try valgrind, maybe it finds some out of array or uninitalized accesses
* look at sb->line which is where edx comes from i think

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

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is