[FFmpeg-devel] [PATCH v2] SH4 mpegaudio decoder optimizations

[Guennadi Liakhovetski]

On Tue, 14 Dec 2010, Reimar D?ffinger wrote:

> On Tue, Dec 14, 2010 at 05:24:22PM +0100, Guennadi Liakhovetski wrote:
> > +#ifndef SUM8_MACS
> > +#define SUM8_MACS(sum, w, p) SUM8(MACS, sum, w, p)
> > +#endif
> > +
> > +#ifndef SUM8_MLSS
> > +#define SUM8_MLSS(sum, w, p) SUM8(MLSS, sum, w, p)
> > +#endif
> > +
> > +#ifndef SUM8P2_MACS_MLSS
> > +#define SUM8P2_MACS_MLSS(sum, sum2, w, w2, p) SUM8P2(sum, MACS, sum2, MLSS, w, w2, p)
> > +#endif
> > +
> > +#ifndef SUM8P2_MLSS_MLSS
> > +#define SUM8P2_MLSS_MLSS(sum, sum2, w, w2, p) SUM8P2(sum, MLSS, sum2, MLSS, w, w2, p)
> > +#endif
> 
> Can't you instead do something like (note I usually get the syntax
> wrong, plus I don't know if there are issues if the generated
> name is a macro as well):
> #define SUM8(op, sum, w, p) SUM8_#op(sum, w, p)

Sorry, I don't see how this can work here. SUM8(...) is already defined 
and that's what most architectures will keep using, or maybe I 
misunderstood you here?

> > +    union {int64_t x; int32_t u32[2];} u =           \
> > +                    {.x = (sum),};                   \
> 
> Does using a union create faster code that doing it "properly"
> with shifts/ors/...?
> To might knowledge that this works is just something that
> gcc "currently" promises, but is not part of the C standard.

Haven't looked, which code is faster, but I don't think there is a way for 
gcc to change this - a lot of software would break, if gcc starts 
interpreting a union of a 64-bit integer and an array of two 32-bit ints 
differently, than now, IMHO.

> > +    : [hi] "+r" (u.u32[1]),                          \
> > +      [lo] "+r" (u.u32[0]),                          \
> > +      [wp] "+r" (wp),                                \
> > +      [pp] "+r" (pp)                                 \
> 
> Do all compilers/compiler versions that are in use support
> named asm arguments?

Do you know any ones, used for SH, where this is not supported? I don't.

> I also think as a paranoia measure these should be +&r
> (but I admit I never understood that so 100%).

hm, don't remember seeing this - early clobber input-output? Does it make 
sense at all? Early clobber tells the compiler, that it cannot reuse an 
input register for this output parameter, and this is anyway the case, if 
it is also an input, isn't it?

> > +#if (!defined(CONFIG_FLOAT) || !CONFIG_FLOAT) && (!defined(FRAC_BITS) || FRAC_BITS > 15)
> 
> I don't think it's valid for those not to be defined?

It is, that's why I added them. They are only defined in 
mpegaudiodec_float.c, and this header is included from many other files.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/