* [PATCH] MIPS/opcodes: Fix alias annotation for negate instructions
@ 2022-03-07 15:18 Sagar Patel
2022-03-07 20:46 ` Maciej W. Rozycki
0 siblings, 1 reply; 4+ messages in thread
From: Sagar Patel @ 2022-03-07 15:18 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: binutils, Sagar Patel
On both regular MIPS and microMIPS 32-bit, the NEG and NEGU assembly
instructions are idioms for SUB and SUBU respectively with the `rs'
operand equal to $0.
Add missing annotation to NEG and NEGU instructions, and create
corresponding test cases.
This fixes `objdump -d -M no-aliases'.
binutils/
* testsuite/binutils-all/mips/micromips-neg-alias.d: New test.
* testsuite/binutils-all/mips/micromips-neg-alias.s: New test
source.
* testsuite/binutils-all/mips/micromips-neg-noalias.d: New test.
* testsuite/binutils-all/mips/mips-neg-alias.d: New test.
* testsuite/binutils-all/mips/mips-neg-alias.s: New test source.
* testsuite/binutils-all/mips/mips-neg-noalias.d: New test.
* testsuite/binutils-all/mips/mips.exp: Run the new tests.
opcodes/
* mips-opc.c (mips_builtin_opcodes): Fix INSN2_ALIAS annotation
for "neg" and "negu" instructions.
* micromips-opc.c (micromips_opcodes): Likewise.
Signed-off-by: Sagar Patel <sagarmp@cs.unc.edu>
---
binutils/ChangeLog | 11 +++++++++++
.../binutils-all/mips/micromips-neg-alias.d | 13 +++++++++++++
.../binutils-all/mips/micromips-neg-alias.s | 14 ++++++++++++++
.../binutils-all/mips/micromips-neg-noalias.d | 13 +++++++++++++
.../testsuite/binutils-all/mips/mips-neg-alias.d | 13 +++++++++++++
.../testsuite/binutils-all/mips/mips-neg-alias.s | 13 +++++++++++++
.../testsuite/binutils-all/mips/mips-neg-noalias.d | 13 +++++++++++++
binutils/testsuite/binutils-all/mips/mips.exp | 4 ++++
opcodes/ChangeLog | 6 ++++++
opcodes/micromips-opc.c | 4 ++--
opcodes/mips-opc.c | 4 ++--
11 files changed, 104 insertions(+), 4 deletions(-)
create mode 100644 binutils/testsuite/binutils-all/mips/micromips-neg-alias.d
create mode 100644 binutils/testsuite/binutils-all/mips/micromips-neg-alias.s
create mode 100644 binutils/testsuite/binutils-all/mips/micromips-neg-noalias.d
create mode 100644 binutils/testsuite/binutils-all/mips/mips-neg-alias.d
create mode 100644 binutils/testsuite/binutils-all/mips/mips-neg-alias.s
create mode 100644 binutils/testsuite/binutils-all/mips/mips-neg-noalias.d
diff --git a/binutils/ChangeLog b/binutils/ChangeLog
index a355e18de74..1f134dbe87f 100644
--- a/binutils/ChangeLog
+++ b/binutils/ChangeLog
@@ -1,3 +1,14 @@
+2022-03-07 Sagar Patel <sagarmp@cs.unc.edu>
+
+ * testsuite/binutils-all/mips/micromips-neg-alias.d: New test.
+ * testsuite/binutils-all/mips/micromips-neg-alias.s: New test
+ source.
+ * testsuite/binutils-all/mips/micromips-neg-noalias.d: New test.
+ * testsuite/binutils-all/mips/mips-neg-alias.d: New test.
+ * testsuite/binutils-all/mips/mips-neg-alias.s: New test source.
+ * testsuite/binutils-all/mips/mips-neg-noalias.d: New test.
+ * testsuite/binutils-all/mips/mips.exp: Run the new tests.
+
2021-03-06 Maciej W. Rozycki <macro@orcam.me.uk>
* testsuite/binutils-all/mips/mips1-branch-alias.d: New test.
diff --git a/binutils/testsuite/binutils-all/mips/micromips-neg-alias.d b/binutils/testsuite/binutils-all/mips/micromips-neg-alias.d
new file mode 100644
index 00000000000..d2bbc00c503
--- /dev/null
+++ b/binutils/testsuite/binutils-all/mips/micromips-neg-alias.d
@@ -0,0 +1,13 @@
+#PROG: objcopy
+#objdump: -d --prefix-addresses --show-raw-insn
+#name: microMIPS negate instruction alias disassembly
+#source: micromips-neg-alias.s
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+[0-9a-f]+ <[^>]*> 0020 0990 neg at,at
+[0-9a-f]+ <[^>]*> 0040 11d0 negu v0,v0
+[0-9a-f]+ <[^>]*> 0080 1990 neg v1,a0
+[0-9a-f]+ <[^>]*> 00a0 31d0 negu a2,a1
+ \.\.\.
diff --git a/binutils/testsuite/binutils-all/mips/micromips-neg-alias.s b/binutils/testsuite/binutils-all/mips/micromips-neg-alias.s
new file mode 100644
index 00000000000..f5122d3b2dc
--- /dev/null
+++ b/binutils/testsuite/binutils-all/mips/micromips-neg-alias.s
@@ -0,0 +1,14 @@
+ .text
+ .set mips32r3
+ .set noat
+ .set noreorder
+ .set micromips
+foo:
+ neg $1
+ negu $2
+ neg $3, $4
+ negu $6, $5
+
+# Force some (non-delay-slot) zero bytes, to make 'objdump' print ...
+ .align 4, 0
+ .space 16
diff --git a/binutils/testsuite/binutils-all/mips/micromips-neg-noalias.d b/binutils/testsuite/binutils-all/mips/micromips-neg-noalias.d
new file mode 100644
index 00000000000..37f4bbd38de
--- /dev/null
+++ b/binutils/testsuite/binutils-all/mips/micromips-neg-noalias.d
@@ -0,0 +1,13 @@
+#PROG: objcopy
+#objdump: -M no-aliases -d --prefix-addresses --show-raw-insn
+#name: microMIPS negate canonical alias disassembly
+#source: micromips-neg-alias.s
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+[0-9a-f]+ <[^>]*> 0020 0990 sub at,zero,at
+[0-9a-f]+ <[^>]*> 0040 11d0 subu v0,zero,v0
+[0-9a-f]+ <[^>]*> 0080 1990 sub v1,zero,a0
+[0-9a-f]+ <[^>]*> 00a0 31d0 subu a2,zero,a1
+ \.\.\.
diff --git a/binutils/testsuite/binutils-all/mips/mips-neg-alias.d b/binutils/testsuite/binutils-all/mips/mips-neg-alias.d
new file mode 100644
index 00000000000..d3da9343e8d
--- /dev/null
+++ b/binutils/testsuite/binutils-all/mips/mips-neg-alias.d
@@ -0,0 +1,13 @@
+#PROG: objcopy
+#objdump: -m mips:3000 -d --prefix-addresses --show-raw-insn
+#name: MIPS1 negate instruction alias disassembly
+#source: mips-neg-alias.s
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+[0-9a-f]+ <[^>]*> 00010822 neg at,at
+[0-9a-f]+ <[^>]*> 00021023 negu v0,v0
+[0-9a-f]+ <[^>]*> 00041822 neg v1,a0
+[0-9a-f]+ <[^>]*> 00053023 negu a2,a1
+ \.\.\.
diff --git a/binutils/testsuite/binutils-all/mips/mips-neg-alias.s b/binutils/testsuite/binutils-all/mips/mips-neg-alias.s
new file mode 100644
index 00000000000..bb7b3c5ac3a
--- /dev/null
+++ b/binutils/testsuite/binutils-all/mips/mips-neg-alias.s
@@ -0,0 +1,13 @@
+ .text
+ .set noat
+ .set noreorder
+ .set mips2
+foo:
+ neg $1
+ negu $2
+ neg $3, $4
+ negu $6, $5
+
+# Force some (non-delay-slot) zero bytes, to make 'objdump' print ...
+ .align 4, 0
+ .space 16
diff --git a/binutils/testsuite/binutils-all/mips/mips-neg-noalias.d b/binutils/testsuite/binutils-all/mips/mips-neg-noalias.d
new file mode 100644
index 00000000000..02d993229c6
--- /dev/null
+++ b/binutils/testsuite/binutils-all/mips/mips-neg-noalias.d
@@ -0,0 +1,13 @@
+#PROG: objcopy
+#objdump: -M no-aliases -m mips:3000 -d --prefix-addresses --show-raw-insn
+#name: MIPS1 negate canonical alias disassembly
+#source: mips-neg-alias.s
+
+.*: +file format .*mips.*
+
+Disassembly of section \.text:
+[0-9a-f]+ <[^>]*> 00010822 sub at,zero,at
+[0-9a-f]+ <[^>]*> 00021023 subu v0,zero,v0
+[0-9a-f]+ <[^>]*> 00041822 sub v1,zero,a0
+[0-9a-f]+ <[^>]*> 00053023 subu a2,zero,a1
+ \.\.\.
diff --git a/binutils/testsuite/binutils-all/mips/mips.exp b/binutils/testsuite/binutils-all/mips/mips.exp
index b6fe11d85a5..c80d0ce819f 100644
--- a/binutils/testsuite/binutils-all/mips/mips.exp
+++ b/binutils/testsuite/binutils-all/mips/mips.exp
@@ -246,6 +246,10 @@ run_dump_test_o32 "mips32r6-branch-alias"
run_dump_test_o32 "mips32r6-branch-noalias"
run_dump_test_o32 "micromips-branch-alias"
run_dump_test_o32 "micromips-branch-noalias"
+run_dump_test_o32 "mips-neg-alias"
+run_dump_test_o32 "mips-neg-noalias"
+run_dump_test_o32 "micromips-neg-alias"
+run_dump_test_o32 "micromips-neg-noalias"
run_dump_test_o32 "mips-note-2"
run_dump_test_n32 "mips-note-2-n32"
diff --git a/opcodes/ChangeLog b/opcodes/ChangeLog
index 98ad3084e43..c7c8ce18835 100644
--- a/opcodes/ChangeLog
+++ b/opcodes/ChangeLog
@@ -1,3 +1,9 @@
+2022-03-07 Sagar Patel <sagarmp@cs.unc.edu>
+
+ * mips-opc.c (mips_builtin_opcodes): Fix INSN2_ALIAS annotation
+ for "neg" and "negu" instructions.
+ * micromips-opc.c (micromips_opcodes): Likewise.
+
2022-03-06 Sagar Patel <sagarmp@cs.unc.edu>
Maciej W. Rozycki <macro@orcam.me.uk>
diff --git a/opcodes/micromips-opc.c b/opcodes/micromips-opc.c
index 9393e22be5b..047b31e4cbb 100644
--- a/opcodes/micromips-opc.c
+++ b/opcodes/micromips-opc.c
@@ -914,8 +914,8 @@ const struct mips_opcode micromips_opcodes[] =
{"mult", "7,s,t", 0x00000cbc, 0xfc003fff, RD_2|RD_3|WR_a, 0, 0, D32, 0 },
{"multu", "s,t", 0x00009b3c, 0xfc00ffff, RD_1|RD_2|WR_HILO, 0, I1, 0, 0 },
{"multu", "7,s,t", 0x00001cbc, 0xfc003fff, RD_2|RD_3|WR_a, 0, 0, D32, 0 },
-{"neg", "d,w", 0x00000190, 0xfc1f07ff, WR_1|RD_2, 0, I1, 0, 0 }, /* sub 0 */
-{"negu", "d,w", 0x000001d0, 0xfc1f07ff, WR_1|RD_2, 0, I1, 0, 0 }, /* subu 0 */
+{"neg", "d,w", 0x00000190, 0xfc1f07ff, WR_1|RD_2, INSN2_ALIAS, I1, 0, 0 }, /* sub 0 */
+{"negu", "d,w", 0x000001d0, 0xfc1f07ff, WR_1|RD_2, INSN2_ALIAS, I1, 0, 0 }, /* subu 0 */
{"neg.d", "T,V", 0x54002b7b, 0xfc00ffff, WR_1|RD_2|FP_D, 0, I1, 0, 0 },
{"neg.s", "T,V", 0x54000b7b, 0xfc00ffff, WR_1|RD_2|FP_S, 0, I1, 0, 0 },
{"neg.ps", "T,V", 0x54004b7b, 0xfc00ffff, WR_1|RD_2|FP_D, 0, I1, 0, 0 },
diff --git a/opcodes/mips-opc.c b/opcodes/mips-opc.c
index 329bdc628cb..c351809cc1f 100644
--- a/opcodes/mips-opc.c
+++ b/opcodes/mips-opc.c
@@ -1620,8 +1620,8 @@ const struct mips_opcode mips_builtin_opcodes[] =
{"multu1", "d,s,t", 0x70000019, 0xfc0007ff, WR_1|RD_2|RD_3|WR_HILO|IS_M, 0, EE, 0, 0 },
{"mulu", "d,v,t", 0x00000099, 0xfc0007ff, WR_1|RD_2|RD_3, 0, I37, 0, 0},
{"mulu", "d,s,t", 0x00000059, 0xfc0007ff, WR_1|RD_2|RD_3|WR_HILO, 0, N5, 0, 0 },
-{"neg", "d,w", 0x00000022, 0xffe007ff, WR_1|RD_2, 0, I1, 0, 0 }, /* sub 0 */
-{"negu", "d,w", 0x00000023, 0xffe007ff, WR_1|RD_2, 0, I1, 0, 0 }, /* subu 0 */
+{"neg", "d,w", 0x00000022, 0xffe007ff, WR_1|RD_2, INSN2_ALIAS, I1, 0, 0 }, /* sub 0 */
+{"negu", "d,w", 0x00000023, 0xffe007ff, WR_1|RD_2, INSN2_ALIAS, I1, 0, 0 }, /* subu 0 */
{"neg.d", "D,V", 0x46200007, 0xffff003f, WR_1|RD_2|FP_D, 0, I1, 0, SF },
{"neg.s", "D,V", 0x46000007, 0xffff003f, WR_1|RD_2|FP_S, 0, I1, 0, 0 },
{"neg.ps", "D,V", 0x46c00007, 0xffff003f, WR_1|RD_2|FP_D, 0, I5_33|IL2F, 0, I37 },
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MIPS/opcodes: Fix alias annotation for negate instructions
2022-03-07 15:18 [PATCH] MIPS/opcodes: Fix alias annotation for negate instructions Sagar Patel
@ 2022-03-07 20:46 ` Maciej W. Rozycki
2022-03-08 6:25 ` Sagar Patel
0 siblings, 1 reply; 4+ messages in thread
From: Maciej W. Rozycki @ 2022-03-07 20:46 UTC (permalink / raw)
To: Sagar Patel; +Cc: binutils
On Mon, 7 Mar 2022, Sagar Patel wrote:
> On both regular MIPS and microMIPS 32-bit, the NEG and NEGU assembly
> instructions are idioms for SUB and SUBU respectively with the `rs'
> operand equal to $0.
Thank you. Your change has passed regression testing against my usual
set of 87 MIPS targets and overall it looks good to me except for a couple
of minor nits, as below.
> diff --git a/binutils/ChangeLog b/binutils/ChangeLog
> index a355e18de74..1f134dbe87f 100644
> --- a/binutils/ChangeLog
> +++ b/binutils/ChangeLog
> @@ -1,3 +1,14 @@
> +2022-03-07 Sagar Patel <sagarmp@cs.unc.edu>
> +
> + * testsuite/binutils-all/mips/micromips-neg-alias.d: New test.
> + * testsuite/binutils-all/mips/micromips-neg-alias.s: New test
> + source.
> + * testsuite/binutils-all/mips/micromips-neg-noalias.d: New test.
> + * testsuite/binutils-all/mips/mips-neg-alias.d: New test.
> + * testsuite/binutils-all/mips/mips-neg-alias.s: New test source.
> + * testsuite/binutils-all/mips/mips-neg-noalias.d: New test.
> + * testsuite/binutils-all/mips/mips.exp: Run the new tests.
> +
Please do not include updates to ChangeLog files with the diff as they
will almost surely cause a conflict right away. Instead please just put
entries intended for ChangeLog files within the change description, as you
correctly did, and then the committer will produce actual ChangeLog file
entries from that text. It could be that eventually we will switch to
doing this automatically just as GCC does or stop using ChangeLog entries
altogether like GDB, which will make backports so much easier.
> diff --git a/binutils/testsuite/binutils-all/mips/micromips-neg-alias.s b/binutils/testsuite/binutils-all/mips/micromips-neg-alias.s
> new file mode 100644
> index 00000000000..f5122d3b2dc
> --- /dev/null
> +++ b/binutils/testsuite/binutils-all/mips/micromips-neg-alias.s
> @@ -0,0 +1,14 @@
> + .text
> + .set mips32r3
> + .set noat
> + .set noreorder
This `.set noreorder' is not needed here (I only put it in the other test
case to avoid clutter from branch delay slots).
FAOD the `.set mips32r3' pseudo-op is required so that configurations
that default to MIPSr6 still assemble this source.
> diff --git a/binutils/testsuite/binutils-all/mips/mips-neg-alias.s b/binutils/testsuite/binutils-all/mips/mips-neg-alias.s
> new file mode 100644
> index 00000000000..bb7b3c5ac3a
> --- /dev/null
> +++ b/binutils/testsuite/binutils-all/mips/mips-neg-alias.s
> @@ -0,0 +1,13 @@
> + .text
> + .set noat
> + .set noreorder
> + .set mips2
Likewise, and also this `.set mips2' is not needed here (I put it in the
other test case so that branch likely instructions, which require the ISA
to be between MIPS II and MIPSr6, assemble correctly).
This might be overly pedantic, but with any test case I suggest putting
the absolute minimum there so as not to make someone in the future wonder
what the specific requirement was for one option or another (possibly I
shouldn't have used `.set noat' and $1 either, but let's leave it as it
is).
It looks perfect otherwise and once your paperwork has been sorted I will
push either your change as posted with amendments made for the two pieces
above or any v2 if you prefer to make one yourself.
Thank you for your submission.
Maciej
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MIPS/opcodes: Fix alias annotation for negate instructions
2022-03-07 20:46 ` Maciej W. Rozycki
@ 2022-03-08 6:25 ` Sagar Patel
2022-03-10 10:11 ` Maciej W. Rozycki
0 siblings, 1 reply; 4+ messages in thread
From: Sagar Patel @ 2022-03-08 6:25 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: binutils
On Mon, Mar 7, 2022 at 3:46 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
> On Mon, 7 Mar 2022, Sagar Patel wrote:
> Please do not include updates to ChangeLog files with the diff as they
> will almost surely cause a conflict right away. Instead please just put
> entries intended for ChangeLog files within the change description, as you
> correctly did, and then the committer will produce actual ChangeLog file
> entries from that text.
Oh, that makes way more sense. I'll drop the changes to the ChangeLog files.
> This might be overly pedantic, but with any test case I suggest putting
> the absolute minimum there so as not to make someone in the future wonder
> what the specific requirement was for one option or another (possibly I
> shouldn't have used `.set noat' and $1 either, but let's leave it as it
> is).
I agree. I can remove the use of `.set noat' in the two new test
sources. Should I do that?
> It looks perfect otherwise and once your paperwork has been sorted I will
> push either your change as posted with amendments made for the two pieces
> above or any v2 if you prefer to make one yourself.
Just to keep you in the loop, I sent in the request for the paperwork.
I'll let you know if I don't hear back soon.
I'll send a v2 with the requested changes. Thank you for the review.
---Sagar Patel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MIPS/opcodes: Fix alias annotation for negate instructions
2022-03-08 6:25 ` Sagar Patel
@ 2022-03-10 10:11 ` Maciej W. Rozycki
0 siblings, 0 replies; 4+ messages in thread
From: Maciej W. Rozycki @ 2022-03-10 10:11 UTC (permalink / raw)
To: Sagar Patel; +Cc: binutils
On Tue, 8 Mar 2022, Sagar Patel wrote:
> > This might be overly pedantic, but with any test case I suggest putting
> > the absolute minimum there so as not to make someone in the future wonder
> > what the specific requirement was for one option or another (possibly I
> > shouldn't have used `.set noat' and $1 either, but let's leave it as it
> > is).
>
> I agree. I can remove the use of `.set noat' in the two new test
> sources. Should I do that?
Please do whatever you prefer and I will accept your choice. This is a
minor difference and I surely cannot impose requirements on you that I do
not stick to myself.
> > It looks perfect otherwise and once your paperwork has been sorted I will
> > push either your change as posted with amendments made for the two pieces
> > above or any v2 if you prefer to make one yourself.
>
> Just to keep you in the loop, I sent in the request for the paperwork.
> I'll let you know if I don't hear back soon.
Ack, thanks.
Maciej
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-03-10 10:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-07 15:18 [PATCH] MIPS/opcodes: Fix alias annotation for negate instructions Sagar Patel
2022-03-07 20:46 ` Maciej W. Rozycki
2022-03-08 6:25 ` Sagar Patel
2022-03-10 10:11 ` Maciej W. Rozycki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).