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

[Michael Niedermayer]

Hi

On Wed, May 30, 2007 at 02:07:53PM -0700, Trent Piepho wrote:
> 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.

sure iam not disagreeing, but most asm will either access things without being
able to tell gcc exactly (->needs "memory" clobber / or luck) or it will
need to have the for/while statement written in C and be slow (->not an
option outside the gcc devel dreamworld)

so your argument is all fine but it fixes half of a problem where the other
half we cannot fix so we only gain compatibility with old as, and as
disadvantage theres the risk that gcc might produce worse code

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070530/0bcc22ea/attachment.pgp>