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

[Zuxy Meng]

Hi,

2007/5/29, Guillaume Poirier <gpoirier at mplayerhq.hu>:
> 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.

It looks good to me, but is the "asm" -> "asm volatile" part
absolutely necessary?
-- 
Zuxy
Beauty is truth,
While truth is beauty.
PGP KeyID: E8555ED6