[committed] MIPS: Fix encoding of sigrie instruction

Maciej W. Rozycki macro@mips.com
Mon Feb 12 14:58:00 GMT 2018
Previous message (by thread): [PATCH] MIPS: Fix encoding of sigrie instruction
Next message (by thread): Linker plugins should be aware of --defsym during symbol resolution
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
From: Henry Wong <henry@stuffedcow.net>

The instruction encoding for the MIPS r6 sigrie instruction seems to be
incorrect.  It's currently 0x4170xxxx (which overlaps with ei, di, evp,
and dvp), but should be 0x0417xxxx.  See ISA reference[1][2].

References:

[1] "MIPS Architecture for Programmers Volume II-A: The MIPS32
    Instruction Set Manual", Imagination Technologies, Inc., Document
    Number: MD00086, Revision 6.06, December 15, 2016, Table A.4 "MIPS32
    REGIMM Encoding of rt Field", p. 452

[2] "MIPS Architecture For Programmers Volume II-A: The MIPS64
    Instruction Set Reference Manual", Imagination Technologies, Inc.,
    Document Number: MD00087, Revision 6.06, December 15, 2016, Table
    A.4 "MIPS64 REGIMM Encoding of rt Field", p. 581

	opcodes/
	* mips-opc.c (mips_builtin_opcodes): Correct "sigrie" encoding.

	gas/
	* testsuite/gas/mips/r6.d: Update for "sigrie" encoding fix.
	* testsuite/gas/mips/r6-n32.d: Likewise.
	* testsuite/gas/mips/r6-n64.d: Likewise.
---
Hi Henry,

 Thank you for your contribution, and for addressing the problem you have 
found.

 Your change looks good except for for minor issues with the patch 
description, which is also missing in the first place from the otherwise 
correctly formed GIT-style patch attached.  I'll address these issues 
below.

 First of all however, please indicate what your legal status is with the 
Free Software Foundation with regard to change submissions to binutils.  
Have you got a copyright assignment or an alternative arrangement in 
place?

 I am asking because together with your previous contribution, commit 
487958d1e995 ("Fix segfault processing nios2 pseudo-instructions with too 
few arguments."), corresponding to your originally submission archived 
here: <https://sourceware.org/ml/binutils/2017-10/msg00012.html>, all your 
code submitted so far to binutils looks to me legally significant, and as 
such requires a legal procedure to follow before we can accept further 
changes from you.

 If you do not have an arrangement with the Free Software Foundation, then 
I'll let you know how to proceed.

 I have decided to commit your change due to its small size and triviality 
(i.e. there's no other way to fix this bug), however for any further 
submissions you'll have to have an arrangement with the Free Software 
Foundation before a change can be accepted.

> The instruction encoding for the MIPS r6 sigrie instruction seems to be
> incorrect. It's currently 0x4170xxxx (which overlaps with ei, di, evp,
> and dvp), but should be 0x0417xxxx (See ISA reference,
> https://www.mips.com/?do-download=the-mips32-instruction-set-v6-06, page
> 385 of the PDF)

 This explanation is good as the patch description, so I have used it, 
with small adjustments, observing that it is instruction bit encoding 
tables rather than bit patterns shown in individual instruction 
descriptions that are normative (we had discrepancies in the past), and 
also referring to architecture specifications by their titles to avoid a 
web link which may turn out volatile in the long term (which happened 
more than once).

 Please avoid submitting patches with only change heading included, except 
for obvious changes, such as documentation typos, formatting fixes, etc.

> The attached patch changes the MIPS opcode table and the tests that are
> affected. I'm not entirely confident there aren't more changes elsewhere
> that need to be made, but gas generates the changed opcode and objdump
> also disassembles the changed opcode correctly.

 Your changes to the test suite indicate that you have actually regression 
tested your change before submitting it.  This is a very good practice 
indeed, but in the future please tell us that you actually did it, 
indicating your target(s) chosen, e.g. `mips-elf', etc.

> Changes:
> 
>     opcodes/mips-opc.c: Update MIPS opcode table
>     gas/testsuite/gas/mips/
>         * r6.d
>         * r6-n32.d
>         * r6-n64.d    Update tests that use sigrie

 ChangeLog entries submitted need to be included at the end of the change 
description, using a fixed format.  In particular you need to include full 
relative paths of all files affected, with the top subdirectory removed 
(i.e. without `opcodes/', `gas/' at the beginning).  You also need to name 
the entity updated, in parentheses, where applicable.  See the change 
description above, existing ChangeLog files and:

<https://www.gnu.org/prep/standards/standards.html#Style-of-Change-Logs>

for a full explanation.

 I have committed your change now, in the form recorded here.

 Thank you,

  Maciej
---
 gas/testsuite/gas/mips/r6-n32.d |    4 ++--
 gas/testsuite/gas/mips/r6-n64.d |    4 ++--
 gas/testsuite/gas/mips/r6.d     |    4 ++--
 opcodes/mips-opc.c              |    2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/gas/testsuite/gas/mips/r6-n32.d b/gas/testsuite/gas/mips/r6-n32.d
index fb55086..c9efa1d 100644
--- a/gas/testsuite/gas/mips/r6-n32.d
+++ b/gas/testsuite/gas/mips/r6-n32.d
@@ -497,6 +497,6 @@ Disassembly of section .text:
 0+0598 <[^>]*> 41600024 	dvp
 0+059c <[^>]*> 41620004 	evp	v0
 0+05a0 <[^>]*> 41620024 	dvp	v0
-0+05a4 <[^>]*> 41700000 	sigrie	0x0
-0+05a8 <[^>]*> 4170ffff 	sigrie	0xffff
+0+05a4 <[^>]*> 04170000 	sigrie	0x0
+0+05a8 <[^>]*> 0417ffff 	sigrie	0xffff
 	\.\.\.
diff --git a/gas/testsuite/gas/mips/r6-n64.d b/gas/testsuite/gas/mips/r6-n64.d
index fd4da21..fa0e86b 100644
--- a/gas/testsuite/gas/mips/r6-n64.d
+++ b/gas/testsuite/gas/mips/r6-n64.d
@@ -753,6 +753,6 @@ Disassembly of section .text:
 0+0598 <[^>]*> 41600024 	dvp
 0+059c <[^>]*> 41620004 	evp	v0
 0+05a0 <[^>]*> 41620024 	dvp	v0
-0+05a4 <[^>]*> 41700000 	sigrie	0x0
-0+05a8 <[^>]*> 4170ffff 	sigrie	0xffff
+0+05a4 <[^>]*> 04170000 	sigrie	0x0
+0+05a8 <[^>]*> 0417ffff 	sigrie	0xffff
 	\.\.\.
diff --git a/gas/testsuite/gas/mips/r6.d b/gas/testsuite/gas/mips/r6.d
index 8588e92..9faa478 100644
--- a/gas/testsuite/gas/mips/r6.d
+++ b/gas/testsuite/gas/mips/r6.d
@@ -496,6 +496,6 @@ Disassembly of section .text:
 0+0598 <[^>]*> 41600024 	dvp
 0+059c <[^>]*> 41620004 	evp	v0
 0+05a0 <[^>]*> 41620024 	dvp	v0
-0+05a4 <[^>]*> 41700000 	sigrie	0x0
-0+05a8 <[^>]*> 4170ffff 	sigrie	0xffff
+0+05a4 <[^>]*> 04170000 	sigrie	0x0
+0+05a8 <[^>]*> 0417ffff 	sigrie	0xffff
 	\.\.\.
diff --git a/opcodes/mips-opc.c b/opcodes/mips-opc.c
index 180d613..b0c6195 100644
--- a/opcodes/mips-opc.c
+++ b/opcodes/mips-opc.c
@@ -1867,7 +1867,7 @@ const struct mips_opcode mips_builtin_opcodes[] =
 {"shfl.repa.qh",	"X,Y,Z",	0x7b20001f, 0xffe0003f, WR_1|RD_2|RD_3|FP_D,	0,		0,		MX,	0 },
 {"shfl.repb.qh",	"X,Y,Z",	0x7ba0001f, 0xffe0003f, WR_1|RD_2|RD_3|FP_D,	0,		0,		MX,	0 },
 {"shfl.upsl.ob",	"X,Y,Z",	0x78c0001f, 0xffe0003f, WR_1|RD_2|RD_3|FP_D,	0,		SB1,		MX,	0 },
-{"sigrie",		"u",		0x41700000, 0xffff0000,	TRAP,			0,		I37,		0,	0 },
+{"sigrie",		"u",		0x04170000, 0xffff0000,	TRAP,			0,		I37,		0,	0 },
 {"sle",			"d,v,t",	0,    (int) M_SLE,	INSN_MACRO,		0,		I1,		0,	0 },
 {"sle",			"d,v,I",	0,    (int) M_SLE_I,	INSN_MACRO,		0,		I1,		0,	0 },
 {"sle",			"S,T",		0x46a0003e, 0xffe007ff,	RD_1|RD_2|WR_CC|FP_D,	0,		IL2E,		0,	0 },
Previous message (by thread): [PATCH] MIPS: Fix encoding of sigrie instruction
Next message (by thread): Linker plugins should be aware of --defsym during symbol resolution
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
More information about the Binutils mailing list