On Fri, Feb 15, 2013 at 03:15:19PM -0500, Daniel Kang wrote:
> On Fri, Feb 15, 2013 at 1:53 PM, Diego Biurrun <[email protected]> wrote:
> > On Fri, Feb 15, 2013 at 12:53:44AM -0500, Daniel Kang wrote:
> > > On Thu, Feb 14, 2013 at 7:59 AM, Diego Biurrun <[email protected]> wrote:
> > > > On Wed, Feb 13, 2013 at 05:53:36PM -0500, Daniel Kang wrote:
> > > >>
> > > >> --- a/libavcodec/x86/cavsdsp.c
> > > >> +++ b/libavcodec/x86/cavsdsp.c
> > > >> @@ -475,12 +481,18 @@ CAVS_MC(put_, 8, 3dnow)
> > > >>  CAVS_MC(put_, 16,3dnow)
> > > >>  CAVS_MC(avg_, 8, 3dnow)
> > > >>  CAVS_MC(avg_, 16,3dnow)
> > > >> +#endif /* HAVE_AMD3DNOW_INLINE */
> > > >>
> > > >>  static av_cold void ff_cavsdsp_init_3dnow(CAVSDSPContext *c,
> > > >>                                            AVCodecContext *avctx)
> > > >>  {
> > > >> +#if HAVE_YASM
> > > >> +    c->put_cavs_qpel_pixels_tab[0][0] =
> > > >> ff_put_cavs_qpel16_mc00_mmxext;
> > > >> +    c->put_cavs_qpel_pixels_tab[1][0] =
> > > >> ff_put_cavs_qpel16_mc00_mmxext;
> > > >> +#endif
> > > >>
> > > >> +#if HAVE_INLINE_ASM
> > > >>  #define dspfunc(PFX, IDX, NUM) \
> > > >> -    c->PFX ## _pixels_tab[IDX][ 0] = ff_ ## PFX ## NUM ##
> > > >> _mc00_mmxext; \
> > > >>      c->PFX ## _pixels_tab[IDX][ 2] = ff_ ## PFX ## NUM ##
> > > >> _mc20_3dnow; \
> > > >>      c->PFX ## _pixels_tab[IDX][ 4] = ff_ ## PFX ## NUM ##
> > > >> _mc01_3dnow; \
> > > >
> > > > mmxext functions in the 3dnow init function?
> > >
> > > Yes this is correct. Does not contain any mmxext specific instructions.
> >
> > That's not my definition of "correct".  It is not wrongly placed, just
> > wrongly named then.  Please fix the name.
> 
> What do you suggest?

It should have a suffix according to the instruction set extension it
uses.  If it does not use mmxext instructions, then either mmx or 3dnow
are the right suffixes, depending on which of the two extension it uses.

> > > >> +    lea          r1, [r1+r2*2]
> > > >> +    lea          r0, [r0+r2*2]
> > > >> +    sub         r3d, 4
> > > >> +    jne .loop
> > > >> +    RET
> > > >
> > > > Weird placement of .loop; I suggest aligning it with the rest.
> > > > Probably it is handled inconsistently throughout...
> > >
> > > All my code I've written has the loop in that placement. It's very
> > > possible it's inconsistent across files.
> >
> > I suggest no idiosyncratic formatting for jump instructions and/or
> > maintaining the style of the file.
> 
> The majority of the code in that file already has loops in that
> format. The exceptions are the two functions at the bottom (the sse2
> ones).

Just do whatever you want.  It's not important.

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to