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

[Trent Piepho]

On Wed, 30 May 2007, Michael Niedermayer wrote:
> On Wed, May 30, 2007 at 07:58:34PM +0100, Patrice Bensoussan wrote:
> > >>
> > >> well these arent the only occurances of this syntax in ffmpeg
> > >> also i would like to see benchmarks, gcc has the tendency to do
> > >> stupid
> > >> things if it can and here it can ... (=more freedom with gcc
> > >> generally means
> > >> worse code, thats just my experience with gcc, its not always
> > >> true, also
> > >> gcc should be getting better as the version numbers increase ...)
> > >> also i dont see how this additional freedom could lead to better code
> > >> here, it can just lead to worse code if gcc doesnt realize that
> > >> things
> > >> can be addressed via the same register
> > >
> > > When I checked the asm output, it generates the exact same
> > > instructions,
> > > just without the semi-incorrect asm syntax.
> > >
> >
> > Hmmm... Michael, the code currently committed is broken for Intel
> > based Macs... so shouldn't we apply this patch anyway to actually
> > have a correct code, and then do some benchmarks later? Or simply
> > revert the original patch until we can decide what is the best thing
> > to do?
>
> wait a moment, what is broken?
> apple ships an ancient broken assembler which silently missassembles
> the code
>
> if apple would silently misscompile i++ we wouldnt start replacing that
> by i=i+1 either ...
>
> replacing all the existing code to workaround that bug (which apple
> will likely fix eventually) shouldnt be done carelessly, its not that

I wouldn't consider it a workaround, but a better way to write the code to
begin with.

The use of the asm("n+%0" :  "m"(foo)) syntax has a bug.  The asm block has an
undeclared input or output.  The fact that the existing code works is simply
luck that gcc's optimizer isn't able to do something to break it.

Consider something like this:
int main(void)
{
    int x[2] = {1, 1};
    asm("movl $42, %0 ; movl $42, 4+%0" : "=m"(x[0]));
    printf("%d\n", x[1]);
}

It should print 42, but it doesn't, with -O2 it prints 1.  The resulting asm:
        movl    $1, -4(%ebp)
#APP
        movl $42, -8(%ebp) ; movl $42, 4+-8(%ebp)
#NO_APP
        pushl   $1
        pushl   $.LC0
        call    printf

Since the asm didn't tell gcc that x[1] was modified, it thinks the value
is still 1.