[PATCH] [inline assembly] fix pcm_dmix_i386.h assembly chunk interfaces

Takashi Iwai tiwai at suse.de
Mon Apr 27 17:12:28 CEST 2020


Please don't drop Cc to ML.

And about the comments:

On Mon, 27 Apr 2020 16:35:32 +0200,
FRÉDÉRIC RECOULES wrote:
> 
> I wrongly assumed that the option -mmmx was passed when compiling (re)
> mix_areas_16_mmx.
> I know how to fix it (inspired of ffmpeg):
>  - the configure test if mmx is supported by the compiler option and set a
> macro HAVE_MMX accordingly (maybe something already exist?).

Actually the mmx support isn't about whether the compiler supports it
or not.  With inline asm, the MMX-enabled code is always included in
alsa-lib as well as the code without MMX, then which one to be
executed is dynamically switched at runtime by probing the CPU
capability.


thanks,

Takashi

>  - declare a macro MMX_CLOBBERS(list) that will output the list when HAVE_MMX
> is true and nothing otherwise.
> I will resubmit a patch soon.
> 
> Regards,
> Frédéric Recoules
> 
> ------------------------------------------------------------------------------
> De: "tiwai" <tiwai at suse.de>
> À: "frederic recoules" <frederic.recoules at univ-grenoble-alpes.fr>
> Cc: "alsa-devel" <alsa-devel at alsa-project.org>, "frederic recoules"
> <frederic.recoules at orange.fr>
> Envoyé: Lundi 27 Avril 2020 15:47:02
> Objet: Re: [PATCH] [inline assembly] fix pcm_dmix_i386.h assembly chunk
> interfaces
> 
> On Mon, 27 Apr 2020 09:36:04 +0200,
> frederic.recoules at univ-grenoble-alpes.fr wrote:
> >
> > From: Frédéric Recoules <frederic.recoules at orange.fr>
> >
> > Main changes are:
> >   - move 'size' and 'old_ebx' to the output list since they are clobbered
> >   - add the "memory" keyword since input pointers are dereferenced
> >   - add mmx registers in the clobber list and add an initialization for mm1
> >   - add ebx in clobbers via a set of macro when GCC is newer than 5.0
> >     (it will work for other compilers or non-PIC mode too)
> >
> > Minor changes are:
> >   - keep consistent the token numbering in the template
> >   - remove the manual save/restore ebx when it is in the clobber list
> >   - allows 'dst_step', 'src_step' and 'sum_step' to be given by immediates
> >   - allows 'size' to be given by register (e.g. ebp)
> >   - add "cc" keyword since the eflag register is clobbered
> >
> > Signed-off-by: Frédéric Recoules <frederic.recoules at orange.fr>
> 
> When I apply this and build for i386 with gcc9, I got the following
> error:
>  pcm_dmix_i386.h: In function 'mix_areas_16_mmx':
>  pcm_dmix_i386.h:180:2: error: unknown register name 'mm1' in 'asm'
>    180 |  __asm__ __volatile__ (
>        |  ^~~~~~~
>  pcm_dmix_i386.h:180:2: error: unknown register name 'mm0' in 'asm'
>  In file included from pcm_dmix_i386.c:31,
>                   from pcm_dmix.c:144:
>  pcm_dmix_i386.h: In function 'remix_areas_16_mmx':
>  pcm_dmix_i386.h:180:2: error: unknown register name 'mm1' in 'asm'
>    180 |  __asm__ __volatile__ (
>        |  ^~~~~~~
> ....
> 
> Could you check those errors?
> 
> thanks,
> 
> Takashi
> 
> > ---
> >  src/pcm/pcm_dmix_i386.h | 168 ++++++++++++++++++++++------------------
> >  1 file changed, 93 insertions(+), 75 deletions(-)
> >
> > diff --git a/src/pcm/pcm_dmix_i386.h b/src/pcm/pcm_dmix_i386.h
> > index 2778cb1d..af2f4630 100644
> > --- a/src/pcm/pcm_dmix_i386.h
> > +++ b/src/pcm/pcm_dmix_i386.h
> > @@ -26,6 +26,13 @@
> >   *
> >   */
> >  
> > +#define COMMA ,
> > +#if __GNUC__ < 5 && defined(__PIC__)
> > +#  define GCC_PIC_SWITCH(before,after) before
> > +#else
> > +#  define GCC_PIC_SWITCH(before,after) after
> > +#endif
> > +
> >  /*
> >   *  for plain i386
> >   */
> > @@ -47,13 +54,14 @@ static void MIX_AREAS_16(unsigned int size,
> >          __asm__ __volatile__ (
> >                  "\n"
> >  
> > -                "\tmovl %%ebx, %7\n"        /* ebx is GOT pointer (-fPIC) *
> /
> > +                /* ebx is GOT pointer (-fPIC) */
> > +                GCC_PIC_SWITCH("\tmovl %%ebx, %1\n",)
> >                  /*
> >                   *  initialization, load ESI, EDI, EBX registers
> >                   */
> > -                "\tmovl %1, %%edi\n"
> > -                "\tmovl %2, %%esi\n"
> > -                "\tmovl %3, %%ebx\n"
> > +                "\tmovl %2, %%edi\n"
> > +                "\tmovl %3, %%esi\n"
> > +                "\tmovl %4, %%ebx\n"
> >                  "\tcmpl $0, %0\n"
> >                  "\tjnz 2f\n"
> >                  "\tjmp 7f\n"
> > @@ -64,9 +72,9 @@ static void MIX_AREAS_16(unsigned int size,
> >                   */
> >                  "\t.p2align 4,,15\n"
> >                  "1:"
> > -                "\tadd %4, %%edi\n"
> > -                "\tadd %5, %%esi\n"
> > -                "\tadd %6, %%ebx\n"
> > +                "\tadd %5, %%edi\n"
> > +                "\tadd %6, %%esi\n"
> > +                "\tadd %7, %%ebx\n"
> >  
> >                  /*
> >                   *   sample = *src;
> > @@ -138,15 +146,16 @@ static void MIX_AREAS_16(unsigned int size,
> >                  "\tjnz 4b\n"
> >                  "\tdecl %0\n"
> >                  "\tjnz 1b\n"
> > -                
> > -                "7:"
> > -                "\tmovl %7, %%ebx\n"        /* ebx is GOT pointer (-fPIC) *
> /
> >  
> > -                : /* no output regs */
> > -                : "m" (size), "m" (dst), "m" (src),
> > -                  "m" (sum), "m" (dst_step), "m" (src_step),
> > -                  "m" (sum_step), "m" (old_ebx)
> > -                : "esi", "edi", "edx", "ecx", "eax"
> > +                "7:"
> > +                /* ebx is GOT pointer (-fPIC) */
> > +                GCC_PIC_SWITCH("\tmovl %1, %%ebx\n",)
> > +
> > +                : "+&rm" (size), GCC_PIC_SWITCH("=m","=X") (old_ebx)
> > +                : "m" (dst), "m" (src), "m" (sum),
> > +                  "im" (dst_step), "im" (src_step), "im" (sum_step)
> > +                : "esi", "edi", "edx", "ecx", GCC_PIC_SWITCH(,"ebx"COMMA)
> "eax",
> > +                  "memory", "cc"
> >          );
> >  }
> >  
> > @@ -171,22 +180,24 @@ static void MIX_AREAS_16_MMX(unsigned int size,
> >          __asm__ __volatile__ (
> >                  "\n"
> >  
> > -                "\tmovl %%ebx, %7\n"        /* ebx is GOT pointer (-fPIC) *
> /
> > +                /* ebx is GOT pointer (-fPIC) */
> > +                GCC_PIC_SWITCH("\tmovl %%ebx, %1\n",)
> >                  /*
> > -                 *  initialization, load ESI, EDI, EBX registers
> > +                 *  initialization, load ESI, EDI, EBX registers, clear MM1
> >                   */
> > -                "\tmovl %1, %%edi\n"
> > -                "\tmovl %2, %%esi\n"
> > -                "\tmovl %3, %%ebx\n"
> > +                "\tpxor %%mm1, %%mm1\n"
> > +                "\tmovl %2, %%edi\n"
> > +                "\tmovl %3, %%esi\n"
> > +                "\tmovl %4, %%ebx\n"
> >                  "\tcmpl $0, %0\n"
> >                  "\tjnz 2f\n"
> >                  "\tjmp 5f\n"
> >  
> >                  "\t.p2align 4,,15\n"
> >                  "1:"
> > -                "\tadd %4, %%edi\n"
> > -                "\tadd %5, %%esi\n"
> > -                "\tadd %6, %%ebx\n"
> > +                "\tadd %5, %%edi\n"
> > +                "\tadd %6, %%esi\n"
> > +                "\tadd %7, %%ebx\n"
> >  
> >                  "2:"
> >                  /*
> > @@ -230,13 +241,14 @@ static void MIX_AREAS_16_MMX(unsigned int size,
> >                  "\tjnz 1b\n"
> >                  "\temms\n"
> >                  "5:"
> > -                "\tmovl %7, %%ebx\n"        /* ebx is GOT pointer (-fPIC) *
> /
> > -
> > -                : /* no output regs */
> > -                : "m" (size), "m" (dst), "m" (src),
> > -                  "m" (sum), "m" (dst_step), "m" (src_step),
> > -                  "m" (sum_step), "m" (old_ebx)
> > -                : "esi", "edi", "edx", "ecx", "eax"
> > +                /* ebx is GOT pointer (-fPIC) */
> > +                GCC_PIC_SWITCH("\tmovl %1, %%ebx\n",)
> > +
> > +                : "+&rm" (size), GCC_PIC_SWITCH("=m","=X") (old_ebx)
> > +                : "m" (dst), "m" (src), "m" (sum),
> > +                  "im" (dst_step), "im" (src_step), "im" (sum_step)
> > +                : "esi", "edi", "edx", "ecx", GCC_PIC_SWITCH(,"ebx"COMMA)
> "eax",
> > +                  "mm0", "mm1", "memory", "cc"
> >          );
> >  }
> >  
> > @@ -261,13 +273,14 @@ static void MIX_AREAS_32(unsigned int size,
> >          __asm__ __volatile__ (
> >                  "\n"
> >  
> > -                "\tmovl %%ebx, %7\n"        /* ebx is GOT pointer (-fPIC) *
> /
> > +                /* ebx is GOT pointer (-fPIC) */
> > +                GCC_PIC_SWITCH("\tmovl %%ebx, %1\n",)
> >                  /*
> >                   *  initialization, load ESI, EDI, EBX registers
> >                   */
> > -                "\tmovl %1, %%edi\n"
> > -                "\tmovl %2, %%esi\n"
> > -                "\tmovl %3, %%ebx\n"
> > +                "\tmovl %2, %%edi\n"
> > +                "\tmovl %3, %%esi\n"
> > +                "\tmovl %4, %%ebx\n"
> >                  "\tcmpl $0, %0\n"
> >                  "\tjnz 1f\n"
> >                  "\tjmp 6f\n"
> > @@ -337,19 +350,20 @@ static void MIX_AREAS_32(unsigned int size,
> >                   */
> >                  "\tdecl %0\n"
> >                  "\tjz 6f\n"
> > -                "\tadd %4, %%edi\n"
> > -                "\tadd %5, %%esi\n"
> > -                "\tadd %6, %%ebx\n"
> > +                "\tadd %5, %%edi\n"
> > +                "\tadd %6, %%esi\n"
> > +                "\tadd %7, %%ebx\n"
> >                  "\tjmp 1b\n"
> > -                
> > -                "6:"
> > -                "\tmovl %7, %%ebx\n"        /* ebx is GOT pointer (-fPIC) *
> /
> >  
> > -                : /* no output regs */
> > -                : "m" (size), "m" (dst), "m" (src),
> > -                  "m" (sum), "m" (dst_step), "m" (src_step),
> > -                  "m" (sum_step), "m" (old_ebx)
> > -                : "esi", "edi", "edx", "ecx", "eax"
> > +                "6:"
> > +                /* ebx is GOT pointer (-fPIC) */
> > +                GCC_PIC_SWITCH("\tmovl %1, %%ebx\n",)
> > +
> > +                : "+&rm" (size), GCC_PIC_SWITCH("=m","=X") (old_ebx)
> > +                : "m" (dst), "m" (src), "m" (sum),
> > +                  "im" (dst_step), "im" (src_step), "im" (sum_step)
> > +                : "esi", "edi", "edx", "ecx", GCC_PIC_SWITCH(,"ebx"COMMA)
> "eax",
> > +                  "memory", "cc"
> >          );
> >  }
> >  
> > @@ -374,13 +388,14 @@ static void MIX_AREAS_24(unsigned int size,
> >          __asm__ __volatile__ (
> >                  "\n"
> >  
> > -                "\tmovl %%ebx, %7\n"        /* ebx is GOT pointer (-fPIC) *
> /
> > +                /* ebx is GOT pointer (-fPIC) */
> > +                GCC_PIC_SWITCH("\tmovl %%ebx, %1\n",)
> >                  /*
> >                   *  initialization, load ESI, EDI, EBX registers
> >                   */
> > -                "\tmovl %1, %%edi\n"
> > -                "\tmovl %2, %%esi\n"
> > -                "\tmovl %3, %%ebx\n"
> > +                "\tmovl %2, %%edi\n"
> > +                "\tmovl %3, %%esi\n"
> > +                "\tmovl %4, %%ebx\n"
> >                  "\tcmpl $0, %0\n"
> >                  "\tjnz 1f\n"
> >                  "\tjmp 6f\n"
> > @@ -443,19 +458,20 @@ static void MIX_AREAS_24(unsigned int size,
> >                   */
> >                  "\tdecl %0\n"
> >                  "\tjz 6f\n"
> > -                "\tadd %4, %%edi\n"
> > -                "\tadd %5, %%esi\n"
> > -                "\tadd %6, %%ebx\n"
> > +                "\tadd %5, %%edi\n"
> > +                "\tadd %6, %%esi\n"
> > +                "\tadd %7, %%ebx\n"
> >                  "\tjmp 1b\n"
> > -                
> > -                "6:"
> > -                "\tmovl %7, %%ebx\n"        /* ebx is GOT pointer (-fPIC) *
> /
> >  
> > -                : /* no output regs */
> > -                : "m" (size), "m" (dst), "m" (src),
> > -                  "m" (sum), "m" (dst_step), "m" (src_step),
> > -                  "m" (sum_step), "m" (old_ebx)
> > -                : "esi", "edi", "edx", "ecx", "eax"
> > +                "6:"
> > +                /* ebx is GOT pointer (-fPIC) */
> > +                GCC_PIC_SWITCH("\tmovl %1, %%ebx\n",)
> > +
> > +                : "+&rm" (size), GCC_PIC_SWITCH("=m","=X") (old_ebx)
> > +                : "m" (dst), "m" (src), "m" (sum),
> > +                  "im" (dst_step), "im" (src_step), "im" (sum_step)
> > +                : "esi", "edi", "edx", "ecx", GCC_PIC_SWITCH(,"ebx"COMMA)
> "eax",
> > +                  "memory", "cc"
> >          );
> >  }
> >  
> > @@ -480,13 +496,14 @@ static void MIX_AREAS_24_CMOV(unsigned int size,
> >          __asm__ __volatile__ (
> >                  "\n"
> >  
> > -                "\tmovl %%ebx, %7\n"        /* ebx is GOT pointer (-fPIC) *
> /
> > +                /* ebx is GOT pointer (-fPIC) */
> > +                GCC_PIC_SWITCH("\tmovl %%ebx, %1\n",)
> >                  /*
> >                   *  initialization, load ESI, EDI, EBX registers
> >                   */
> > -                "\tmovl %1, %%edi\n"
> > -                "\tmovl %2, %%esi\n"
> > -                "\tmovl %3, %%ebx\n"
> > +                "\tmovl %2, %%edi\n"
> > +                "\tmovl %3, %%esi\n"
> > +                "\tmovl %4, %%ebx\n"
> >                  "\tcmpl $0, %0\n"
> >                  "\tjz 6f\n"
> >  
> > @@ -541,19 +558,20 @@ static void MIX_AREAS_24_CMOV(unsigned int size,
> >                  /*
> >                   * while (size-- > 0)
> >                   */
> > -                "\tadd %4, %%edi\n"
> > -                "\tadd %5, %%esi\n"
> > -                "\tadd %6, %%ebx\n"
> > +                "\tadd %5, %%edi\n"
> > +                "\tadd %6, %%esi\n"
> > +                "\tadd %7, %%ebx\n"
> >                  "\tdecl %0\n"
> >                  "\tjnz 1b\n"
> > -                
> > -                "6:"
> > -                "\tmovl %7, %%ebx\n"        /* ebx is GOT pointer (-fPIC) *
> /
> >  
> > -                : /* no output regs */
> > -                : "m" (size), "m" (dst), "m" (src),
> > -                  "m" (sum), "m" (dst_step), "m" (src_step),
> > -                  "m" (sum_step), "m" (old_ebx)
> > -                : "esi", "edi", "edx", "ecx", "eax"
> > +                "6:"
> > +                /* ebx is GOT pointer (-fPIC) */
> > +                GCC_PIC_SWITCH("\tmovl %1, %%ebx\n",)
> > +
> > +                : "+&rm" (size), GCC_PIC_SWITCH("=m","=X") (old_ebx)
> > +                : "m" (dst), "m" (src), "m" (sum),
> > +                  "im" (dst_step), "im" (src_step), "im" (sum_step)
> > +                : "esi", "edi", "edx", "ecx", GCC_PIC_SWITCH(,"ebx"COMMA)
> "eax",
> > +                  "memory", "cc"
> >          );
> >  }
> > --
> > 2.17.1
> >
> 
> 


More information about the Alsa-devel mailing list