[FFmpeg-devel] r9017 breaks WMA decoding on Intel Macs

[Guillaume Poirier]

Hi folks,
Le 28 mai 07 ? 01:26, Trent Piepho a ?crit :

> On Mon, 28 May 2007, Guillaume POIRIER wrote:
>> On 5/28/07, Trent Piepho <xyzzy at speakeasy.org> wrote:
>>> On Sun, 27 May 2007, Guillaume POIRIER wrote:
>>>> On 5/27/07, Zuxy Meng <zuxy.meng at gmail.com> wrote:
>>>>> 2007/5/27, Guillaume Poirier <gpoirier at mplayerhq.hu>:
>>>>
>>>>> Then please check things like "8+%0" "-16+%1", replace constraints
>>>>> from "m" to "r" and rewrite using "8(%0)" "-16(%1)". Maybe Apple's
>>>>> binutils doesn't like such syntax.
>>>>
>>>> I guess I suck at doing such things too. Attached patch is an  
>>>> attempt
>>>> to do what you suggest, be it doesn't assemble, either on Linux  
>>>> x86-64
>>>> or OSX x86
>>>
>>> When you change from "m" to "r" you're also changing from the  
>>> lvalue itself
>>> to a pointer to the lvalue.
>>>
>>> This results in less efficient code, since you preclude using SIB
>>> addressing and might need an extra register.  If the apple  
>>> version of gas
>>> doesn't like the syntax, it should generate an error.
>>
>>
>> The said syntax seems to be supported, since there's no error
>> reported. Trent, you mean that it shouldn't be necessary to replace
>> "8+%0" "-16+%1", to "8(%0)" "-16(%1)" ?
>>
>> When I do this change, and this change alone (no "m" to "r"  
>> contrain),
>> it doesn't assemble either.
>
> In general you can write:
>  int x, foo[16];
>  asm("mov 4+%1, %0" : "r"(x) : "m"(foo[4]));
>
> Or you can write this:
>  int x, foo[16];
>  asm("mov 4(%1), %0" : "r"(x) : "r"(foo + 4));
>  asm("mov 4(%1), %0" : "r"(x) : "r"(&foo[4]));  // same as previous  
> line
>
> The first form, with "4+%1" is better.  It can result in more  
> efficient
> code, and is some cases use one less register, which can be very  
> important.
>
> Both ways of writing this are slightly broken though.  If you also  
> touch
> foo from some C code, the optimizer could produce incorrect code.   
> This
> is better:
>  int x, foo[16];
>  asm("mov %2, %0" : "r"(x) : "m"(foo[4]), "m"(foo[4+1]));
>
> I'm attaching a patch showing the right way to do.

Ok, I get the idea. Inline asm is pretty much black magic to me.

Your patch doesn't work, as it doesn't even assemble, but attached  
corrected patch does fix the problem.

Zuxy, Michael, what do you think about it?
Note that I did try to putting all asm blocks of fft_sse.c as "asm  
volatile" but that didn't fix the problem alone.

Guillaume
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: fft_sse.c-fix-for-OSX.0.txt
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070529/03674351/attachment.txt>
-------------- next part --------------