Re: [PATCH] MIPS: Correct opcode for some IL3A instructions

["Maciej W. Rozycki" <macro at mips dot com>]

Hi Jiaxun,

 Thank you for your contribution.

 I will be happy to accept your change, however I need a better 
documentation reference as the justification.  There are a couple of minor 
issues as well.  See below for details.

> Correct instrotions' opcode according to Loongson 3A2000's user manual.
> Ref(in Chinese): http://www.loongson.cn/uploadfile/cpu/3A2000/Loongson3A2000_user2.pdf

 Are you sure it is the right reference though?  I looked through the 
manual and I cannot find encodings for these instructions shown anywhere.  
Granted, I might well be missing something, given my regrettable lack of 
skills in Chinese, so I will appreciate a specific page pointer if indeed 
the required information is there and its only me unable to find it.

 In future submissions please wrap the description so as to make lines fit 
in 74 columns (and I recommend 72 columns, due to how indentation works 
with `git show', etc.).  Also two spaces are required after full stops.  
For a complete explanation of the rules see:

<https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards#Formatting_Your_Code>

and:

<https://www.gnu.org/prep/standards/standards.html#Formatting>.

> ChangeLog:
> 
> opcodes/
> 	* mips-opc.c
> 	(mips_opcodes): Fix opcodes gslwlec1,
> 	 gslwgtc1, gsldlec1, gsldgtc1.

 I am glad that you have included a ChangeLog entry, which is required.  
Your formatting is mostly good, but it has been unnecessarily wrapped.  
The 74 (or 72) column rule applies here as well, so:

	* mips-opc.c (mips_opcodes): Fix opcodes gslwlec1, gslwgtc1, 
	gsldlec1, gsldgtc1.

would be my recommendation (a single leading tab and no further 
indentation for subsequent lines).

> diff --git a/opcodes/mips-opc.c b/opcodes/mips-opc.c
> index 180d613c93..248aab5124 100644
> --- a/opcodes/mips-opc.c
> +++ b/opcodes/mips-opc.c
> @@ -470,10 +470,10 @@ const struct mips_opcode mips_builtin_opcodes[] =
>  {"gsswgt",		"t,b,d",	0xe8000015, 0xfc0007ff,	RD_1|RD_2|RD_3|SM,	0,		IL3A,		0,	0 },
>  {"gssdle",		"t,b,d",	0xe8000016, 0xfc0007ff,	RD_1|RD_2|RD_3|SM,	0,		IL3A,		0,	0 },
>  {"gssdgt",		"t,b,d",	0xe8000017, 0xfc0007ff,	RD_1|RD_2|RD_3|SM,	0,		IL3A,		0,	0 },
> -{"gslwlec1",		"T,b,d",	0xc8000018, 0xfc0007ff,	WR_1|RD_2|RD_3|LM,	0,		IL3A,		0,	0 },
> -{"gslwgtc1",		"T,b,d",	0xc8000019, 0xfc0007ff,	WR_1|RD_2|RD_3|LM,	0,		IL3A,		0,	0 },
> -{"gsldlec1",		"T,b,d",	0xc800001a, 0xfc0007ff,	WR_1|RD_2|RD_3|LM,	0,		IL3A,		0,	0 },
> -{"gsldgtc1",		"T,b,d",	0xc800001b, 0xfc0007ff,	WR_1|RD_2|RD_3|LM,	0,		IL3A,		0,	0 },
> +{"gslwlec1",		"T,b,d",	0xc800001c, 0xfc0007ff,	WR_1|RD_2|RD_3|LM,	0,		IL3A,		0,	0 },
> +{"gslwgtc1",		"T,b,d",	0xc800001d, 0xfc0007ff,	WR_1|RD_2|RD_3|LM,	0,		IL3A,		0,	0 },
> +{"gsldlec1",		"T,b,d",	0xc800001e, 0xfc0007ff,	WR_1|RD_2|RD_3|LM,	0,		IL3A,		0,	0 },
> +{"gsldgtc1",		"T,b,d",	0xc800001f, 0xfc0007ff,	WR_1|RD_2|RD_3|LM,	0,		IL3A,		0,	0 },
>  {"gsswlec1",		"T,b,d",	0xe800001c, 0xfc0007ff,	RD_1|RD_2|RD_3|SM,	0,		IL3A,		0,	0 },
>  {"gsswgtc1",		"T,b,d",	0xe800001d, 0xfc0007ff,	RD_1|RD_2|RD_3|SM,	0,		IL3A,		0,	0 },
>  {"gssdlec1",		"T,b,d",	0xe800001e, 0xfc0007ff,	RD_1|RD_2|RD_3|SM,	0,		IL3A,		0,	0 },

 This is fine by itself, as long as you are able to provide me with a 
backing documentation reference I requested above.

 However this causes a testsuite regression:

FAIL: Loongson-3A tests

which also needs to be corrected, in gas/testsuite/gas/mips/loongson-3a.d.

 Please always regression-test any changes you propose -- it is as easy as 
running `make check' after building binutils and watching out for changes 
in the output produced compared to the output obtained from an unpatched 
build.  Detailed logs will be provided in: binutils/binutils.log, 
gas/testsuite/gas.log and ld/ld.log, for the binutils, GAS and LD subsets 
of tests respectively.  Substitute .sum for .log as the file suffix for 
test summaries.  Let me know if you need further assistance with 
regression testing.

 Since this is one of your first submissions to binutils, I will be happy 
to correct all these issues for you to give you guidance examples for the 
future, however before I go ahead and commit your change I need that 
documentation reference, so please try to locate one and come back to me.  

 I'll be looking into your other submission shortly.

  Maciej