Re: [PATCH]: x86 gas: allow 'rep' prefix on 'bsf' and 'bsr' instructions

["Jan Beulich" <JBeulich at suse dot com>]

>>> On 21.06.12 at 23:36, Roland McGrath <mcgrathr@google.com> wrote:
> 'rep; bsf ...'/'rep; bsr ...' are encoded the same as 'tzcnt ...'/'lzcnt 
> ...'.

While tzcnt really is an extension of bsf, lzcnt is not one of bsr,
so I don't really follow why it's useful to allow the prefix there.

Jan

> When not doing -mbmi, GCC (trunk) like to emit 'rep; bsf ...' on the
> theory that since the two instructions have sufficiently similar
> semantics for the purposes for which the compiler emits this,
> BMI-capable hardware will decode it as 'tzcnt ...' and may execute that
> faster than 'bsf ...', while older hardware will ignore the REP prefix
> and decode it as 'bsf ...'.
> 
> When using .bundle_align_mode, the assembler might decide to insert some
> nop padding between any two instructions, so the ';' could become some
> number of nop instructions and break the encoding intended.
> 
> This change makes the assembler accept 'rep bsf ...' or 'rep bsr ...'
> without complaint.  The result is the same as using the 'tzcnt' or
> 'lzcnt' mnemonic, but the 'rep' forms are accepted even under
> -march=i686 or the like where 'tzcnt' and 'lzcnt' would be refused.
> 
> With this, I can change the compiler to use this syntax (when configured
> with a new assembler) and remove the possibility of running afoul of
> .bundle_align_mode nop-insertion.
> 
> No testsuite failures on an x86_64-linux-gnu host.
> 
> 
> Ok for trunk?
> 
> (I've omitted the large patch to the generated file opcodes/i386-tbl.h,
> so use --enable-maintainer-mode to test the patch.)
> 
> 
> Thanks,
> Roland
> 
> 
> gas/
> 2012-06-21  Roland McGrath  <mcgrathr@google.com>
> 
> 	* config/tc-i386.c (parse_insn): Don't complain about REP prefix
> 	when the template has opcode_modifier.repprefixok set.
> 	* NEWS: Mention the change.
> 
> opcodes/
> 2012-06-21  Roland McGrath  <mcgrathr@google.com>
> 
> 	* i386-opc.h (RepPrefixOk): New enum constant.
> 	(i386_opcode_modifier): New bitfield 'repprefixok'.
> 	* i386-gen.c (opcode_modifiers): Add RepPrefixOk.
> 	* i386-opc.tbl (bsf, bsr): Add RepPrefixOk.
> 	* i386-tbl.h: Regenerate.
> 
> diff --git a/gas/NEWS b/gas/NEWS
> index 6b6dbba..6f62b93 100644
> --- a/gas/NEWS
> +++ b/gas/NEWS
> @@ -13,6 +13,8 @@
>  
>  * Add support for the Adapteva EPIPHANY architecture.
>  
> +* For x86, allow 'rep bsf' or 'rep bsr' syntax.
> +
>  Changes in 2.22:
>  
>  * Add support for the Tilera TILEPro and TILE-Gx architectures.
> diff --git a/gas/config/tc-i386.c b/gas/config/tc-i386.c
> index d2b4927..e3cdf71 100644
> --- a/gas/config/tc-i386.c
> +++ b/gas/config/tc-i386.c
> @@ -3534,7 +3534,7 @@ skip:
>        static templates override;
>  
>        for (t = current_templates->start; t < current_templates->end; ++t)
> -	if (t->opcode_modifier.isstring)
> +	if (t->opcode_modifier.isstring || t->opcode_modifier.repprefixok)
>  	  break;
>        if (t >= current_templates->end)
>  	{
> @@ -3543,7 +3543,7 @@ skip:
>  	  return NULL;
>  	}
>        for (override.start = t; t < current_templates->end; ++t)
> -	if (!t->opcode_modifier.isstring)
> +	if (!t->opcode_modifier.isstring && !t->opcode_modifier.repprefixok)
>  	  break;
>        override.end = t;
>        current_templates = &override;
> diff --git a/opcodes/i386-gen.c b/opcodes/i386-gen.c
> index 21f600f..9386a97 100644
> --- a/opcodes/i386-gen.c
> +++ b/opcodes/i386-gen.c
> @@ -1,4 +1,4 @@
> -/* Copyright 2007, 2008, 2009, 2010, 2011
> +/* Copyright 2007, 2008, 2009, 2010, 2011, 2012
>     Free Software Foundation, Inc.
>  
>     This file is part of the GNU opcodes library.
> @@ -394,6 +394,7 @@ static bitfield opcode_modifiers[] =
>    BITFIELD (No_ldSuf),
>    BITFIELD (FWait),
>    BITFIELD (IsString),
> +  BITFIELD (RepPrefixOk),
>    BITFIELD (IsLockable),
>    BITFIELD (RegKludge),
>    BITFIELD (FirstXmm0),
> diff --git a/opcodes/i386-opc.h b/opcodes/i386-opc.h
> index f130a96..f761770 100644
> --- a/opcodes/i386-opc.h
> +++ b/opcodes/i386-opc.h
> @@ -1,5 +1,5 @@
>  /* Declarations for Intel 80386 opcode table
> -   Copyright 2007, 2008, 2009, 2010
> +   Copyright 2007, 2008, 2009, 2010, 2012
>     Free Software Foundation, Inc.
>  
>     This file is part of the GNU opcodes library.
> @@ -290,6 +290,8 @@ enum
>    FWait,
>    /* quick test for string instructions */
>    IsString,
> +  /* additional instruction on which a "rep" prefix is acceptable */
> +  RepPrefixOk,
>    /* quick test for lockable instructions */
>    IsLockable,
>    /* fake an extra reg operand for clr, imul and special register
> @@ -438,6 +440,7 @@ typedef struct i386_opcode_modifier
>    unsigned int no_ldsuf:1;
>    unsigned int fwait:1;
>    unsigned int isstring:1;
> +  unsigned int repprefixok:1;
>    unsigned int islockable:1;
>    unsigned int regkludge:1;
>    unsigned int firstxmm0:1;
> diff --git a/opcodes/i386-opc.tbl b/opcodes/i386-opc.tbl
> index 8a43b51..8a25e15 100644
> --- a/opcodes/i386-opc.tbl
> +++ b/opcodes/i386-opc.tbl
> @@ -1,5 +1,5 @@
>  // i386 opcode table.
> -// Copyright 2007, 2008, 2009, 2010
> +// Copyright 2007, 2008, 2009, 2010, 2012
>  // Free Software Foundation, Inc.
>  //
>  // This file is part of the GNU opcodes library.
> @@ -475,8 +475,8 @@ xlat, 0, 0xd7, None, 1, 0, 
> No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString, {
>  xlat, 1, 0xd7, None, 1, 0, 
> No_wSuf|No_lSuf|No_sSuf|No_qSuf|No_ldSuf|IsString, { 
> Byte|Unspecified|BaseIndex|Disp8|Disp16|Disp32|Disp32S }
>  
>  // Bit manipulation.
> -bsf, 2, 0xfbc, None, 2, Cpu386, Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf, 
> { 
> Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|
> Disp32S, Reg16|Reg32|Reg64 }
> -bsr, 2, 0xfbd, None, 2, Cpu386, Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf, 
> { 
> Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|
> Disp32S, Reg16|Reg32|Reg64 }
> +bsf, 2, 0xfbc, None, 2, Cpu386, 
> Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf|RepPrefixOk, { 
> Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|
> Disp32S, Reg16|Reg32|Reg64 }
> +bsr, 2, 0xfbd, None, 2, Cpu386, 
> Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf|RepPrefixOk, { 
> Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|
> Disp32S, Reg16|Reg32|Reg64 }
>  bt, 2, 0xfa3, None, 2, Cpu386, Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf, 
> { Reg16|Reg32|Reg64, 
> Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|
> Disp32S }
>  bt, 2, 0xfba, 0x4, 2, Cpu386, Modrm|No_bSuf|No_sSuf|No_ldSuf, { Imm8, 
> Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|
> Disp32S }
>  btc, 2, 0xfbb, None, 2, Cpu386, 
> Modrm|CheckRegSize|No_bSuf|No_sSuf|No_ldSuf|IsLockable|HLEPrefixOk, { 
> Reg16|Reg32|Reg64, 
> Reg16|Reg32|Reg64|Word|Dword|Qword|Unspecified|BaseIndex|Disp8|Disp16|Disp32|
> Disp32S }