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

[Guillaume Poirier]

Hi,

castet.matthieu at free.fr wrote:
> Quoting Guillaume POIRIER <poirierg at gmail.com>:
> 
>> Hi,
>>
>>> This won't assemble correctly if any of the memory operands already have a
>>> displacement. Loren explained why:
>>> http://article.gmane.org/gmane.comp.video.ffmpeg.devel/51453
>> I agree. This patch's main purpose is to show how to write this
>> routine with proper syntax
>
> No, it is broken if you aren't lucky (ie the offset is positive).

NO! Read the message I posted on Date: Wed, 30 May 2007 00:28:58 +0200
 with the dissasm.
Here's a snippey:

Linux:
 1bd:   0f 28 02                movaps (%edx),%xmm0
 1c0:   0f 28 19                movaps (%ecx),%xmm3
 1c3:   0f 28 62 f0             movaps 0xfffffff0(%edx),%xmm4
 1c7:   0f 28 79 10             movaps 0x10(%ecx),%xmm7


OSX:
000001d7        movaps  (%ebx),%xmm0
000001da        movaps  (%edi),%xmm3
000001dd        movaps  0x00(%ebx),%xmm4
000001e1        movaps  0x00(%edi),%xmm7

As you can see _all_ offsets (both positive and negative, note that
0xfffffff0 is -16) are discarded.


>> (Bear in mind that all assemblers produce a
>> warning here. To a certain extend, we may consider that the reason it
>> works with FSF AS is just by pure luck.)
>
> Well that's more like a gas extension that was made for case like that.

Maybe, but even on MacIntel, it's GAS. It's just based on an older
version that supports Mach-O:
as -version
Apple Computer, Inc. version cctools-622.5.obj~13, GNU
assembler version 1.38

>> But IMHO, it's a bit pointless, because
>> whatever the speed figures may look like, we are comparing 1 solution
>> that appears to work by luck, and another that is more reliable. Speed
>> isn't what your patch is after.
>
> There is no luck in the old solution providing :
> - we tell gcc the memory we modify (may be using "memory" clobber).
> - we use a gas supporting the +(%reg) syntax

I disagree. Newer gas _do_ complain about the syntax, and Trent
already explained the shortcomings of current implementation, no need
for me to restate them here.

Guillaume