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

[Martin von Gagern]

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).

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.

Greetings,
 Martin von Gagern
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: snowdsp_mmx-4.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20060810/7e33d17c/attachment.txt>