Re: [PATCH v2] Work around the NOP issue of Loongson2F

["Maciej W. Rozycki" <macro at linux-mips dot org>]

On Mon, 16 Nov 2009, Wu Zhangjin wrote:

> 2009-11-13 Wu Zhangjin <wuzhangjin@gmail.com>, Lemote Inc.
> 
> 	* opcodes/mips-opc.c (loongson2f_nop_insn): New variable.
> 	(mips_builtin_opcodes): Add a NOP macro instruction
> 	* include/opcde/mips.h (loongson2f_nop_insn): Declare variable.
> 	(M_NOP): New enum.
> 	* gas/config/tc-mips.c (mips_fix_loongson2f_nop): New variable.
> 	(md_parse_option): Initialize mips_fix_loongson2f_nop when
> 	-mfix-loongson2f-nop/-mno-fix-loongson2f-nop is passed.
> 	(md_begin): Initialize nop_insn from loongson2f_nop_insn
> 	when mips_fix_loongson2f_nop is true.
> 	(macro): Expand the NOP macro to loongson2f_nop_insn when
> 	mips_fix_loongson2f_nop is true.
> 	* gas/doc/c-mips.texi: Document -mfix-loongson2f-nop
> 	* gas/testsuite/gas/mips/loongson-2f-2.s: New test of
> 	-mfix-loongson2f-nop.
> 	* gas/testsuite/gas/mips/loongson-2f-2.d: Likewise.
> 	* gas/testsuite/gas/mips/mips.exp: Run the loongson-2f-2 test.

 Per Richard's suggestion (which I second) please name the option 
-mfix-loongson2f and adjust all the corresponding variables, etc. 
accordingly.

> @@ -11312,6 +11331,8 @@ struct option md_longopts[] =
>    {"mno-fix-vr4120", no_argument, NULL, OPTION_NO_FIX_VR4120},
>    {"mfix-vr4130",    no_argument, NULL, OPTION_FIX_VR4130},
>    {"mno-fix-vr4130", no_argument, NULL, OPTION_NO_FIX_VR4130},
> +  {"mfix-loongson2f-nop", no_argument, NULL, OPTION_FIX_LOONGSON2F_NOP},
> +  {"mno-fix-loongson2f-nop", no_argument, NULL, OPTION_NO_FIX_LOONGSON2F_NOP},
>    {"mfix-24k",    no_argument, NULL, OPTION_FIX_24K},
>    {"mno-fix-24k", no_argument, NULL, OPTION_NO_FIX_24K},
>  

 Hmm, the -mfix-<foo> options should really be listed alphabetically, but 
failing that I suggest you place yours last rather than between the 
existing ones.  Likewise elsewhere.

> --- /dev/null
> +++ b/gas/testsuite/gas/mips/loongson-2f-2.s
> @@ -0,0 +1,10 @@
> +# Test workarounds selected by -mfix-loongson2f-nop
> +
> +	.text
> +	.set noreorder
> +
> +	nop
> +	b	1f
> +	nop
> +1:
> +	nop

 Please add trailing padding like with many other MIPS tests -- look for a 
.fill directive with the associated comment at the bottom of one of them 
and copy the fragment over.  This is so that padding that may be added by 
GAS implicitly to satisfy varying code alignment requirements across 
targets does not make the test fail for some of them.

  Maciej