[Ffmpeg-devel] libswscale coding style

[Marc Hoffman]

Michael Niedermayer writes:
 > Hi
 > 
 > On Sat, Apr 28, 2007 at 01:20:17AM +0200, Diego Biurrun wrote:
 > > On Fri, Apr 27, 2007 at 07:14:30PM +0200, Diego Biurrun wrote:
 > > > On Fri, Apr 27, 2007 at 12:29:28PM +0200, Ivo wrote:
 > > > > 
 > > > > On Friday 27 April 2007 03:23, Marc Hoffman wrote:
 > > > > > Diego Biurrun writes:
 > > > > >  > I've just implemented pre-commit hooks for libswscale that check for
 > > > > >  > tabs and trailing whitespace, same as for FFmpeg.
 > > > > >  >
 > > > > >  > I'm in the process of removing trailing whitespace and tabs and
 > > > > >  > reformatting the files while I'm at it.
 > > > > >  > What's the preferred style for inline ASM, things like
 > > > > >  > rgb2rgb_template.c:100?
 > > > > 
 > > > > I would really like to see the "instr op, op\n\t" be replaced by something 
 > > > > more readable, like:
 > > > > 
 > > > >     "   movq %%mm0, %%mm1       \n"
 > > > >     "   movq %%mm1, %%mm2       \n"
 > > > >     "   punpckldq 9%1, %%mm1    \n"
 > > > > 
 > > > > et cetera. Perhaps even align the operands, although that'll be a lot of 
 > > > > work.
 > > > 
 > > > I think I'll align them while I'm at it.  What's the preferred style?
 > > 
 > > Let me be more specific:
 > > 
 > > variant 1)
 > > 
 > >   __asm __volatile(
 > >       PREFETCH"        32%1           \n\t"
 > >       "movd              %1, %%mm0    \n\t"
 > >       "punpckldq        3%1, %%mm0    \n\t"
 > >       "movd             6%1, %%mm1    \n\t"
 > >       "punpckldq        9%1, %%mm1    \n\t"
 > >       "movd            12%1, %%mm2    \n\t"
 > >       "punpckldq       15%1, %%mm2    \n\t"
 > >       "movd            18%1, %%mm3    \n\t"
 > >       "punpckldq       21%1, %%mm3    \n\t"
 > >       "pand           %%mm7, %%mm0    \n\t"
 > >       "pand           %%mm7, %%mm1    \n\t"
 > >       "pand           %%mm7, %%mm2    \n\t"
 > >       "pand           %%mm7, %%mm3    \n\t"
 > >       MOVNTQ"         %%mm0,   %0     \n\t"
 > >       MOVNTQ"         %%mm1,  8%0     \n\t"
 > >       MOVNTQ"         %%mm2, 16%0     \n\t"
 > >       MOVNTQ"         %%mm3, 24%0"
 > >       :"=m"(*dest)
 > >       :"m"(*s)
 > >       :"memory");
 > > 
 > > 
 > > or variant 2)
 > > 
 > >   __asm __volatile(
 > >       PREFETCH"       32%1            \n\t"
 > >       "movd           %1,    %%mm0    \n\t"
 > >       "punpckldq      3%1,   %%mm0    \n\t"
 > >       "movd           6%1,   %%mm1    \n\t"
 > >       "punpckldq      9%1,   %%mm1    \n\t"
 > >       "movd           12%1,  %%mm2    \n\t"
 > >       "punpckldq      15%1,  %%mm2    \n\t"
 > >       "movd           18%1,  %%mm3    \n\t"
 > >       "punpckldq      21%1,  %%mm3    \n\t"
 > >       "pand           %%mm7, %%mm0    \n\t"
 > >       "pand           %%mm7, %%mm1    \n\t"
 > >       "pand           %%mm7, %%mm2    \n\t"
 > >       "pand           %%mm7, %%mm3    \n\t"
 > >       MOVNTQ"         %%mm0, %0       \n\t"
 > >       MOVNTQ"         %%mm1, 8%0      \n\t"
 > >       MOVNTQ"         %%mm2, 16%0     \n\t"
 > >       MOVNTQ"         %%mm3, 24%0"
 > >       :"=m"(*dest)
 > >       :"m"(*s)
 > >       :"memory");
 > > 
 > > ?
 > 
 > variant 1 looks better

Hmm very subtle...but I guess I agree 1 does look better.