This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[PATCH] MIPS16, delay slot filling, assembly listing files
- From: Matthew Fortune <Matthew dot Fortune at imgtec dot com>
- To: "binutils at sourceware dot org" <binutils at sourceware dot org>
- Cc: Maciej Rozycki <Maciej dot Rozycki at imgtec dot com>
- Date: Tue, 14 Jun 2016 14:36:57 +0000
- Subject: [PATCH] MIPS16, delay slot filling, assembly listing files
- Authentication-results: sourceware.org; auth=none
The following test fails with an assert in append_insn when built with:
mips...as -mips32r2 -mips16 -O2 -aln=test.lst
hello:
addiu $4, $4, 4
jr $31
When generating a listing file gas seems to place every instruction in
its own fragment which breaks the 'rule' that MIPS16 delay slot and
jump must be in the same fragment. I don't know if the requirement
to have these in the same fragment is valid or not but I looked at
supporting them being in different fragments. The logic in this area
seems rather delicate so I'm not sure of the fix.
After following all the logic through for the various cases in
APPEND_SWAP it seems there is nothing different about the MIPS16 case
and the normal case for when DS and jump are in the same fragment.
There is also nothing obviously wrong with the code for mis-matching
fragments if applied to MIPS16.
So... My best guess at a fix is to remove the MIPS16 special case and
use the same code as MIPS32 and microMIPS.
Relevant history in this area are three commits (most recent first):
a4e064680 <no useful one-liner>
b8ee1a6e8 <no useful one-liner>
e9df6573b <no useful one-liner>
And a mailing list discussion at:
https://www.sourceware.org/ml/binutils/2005-09/msg00005.html
thanks,
Matthew
gas/
* config/tc-mips.c (append_insn): Do not special case MIPS16
for the APPEND_SWAP case.
* testsuite/gas/mips/lst.s: New file.
* testsuite/gas/mips/mips32-lst.d: Likewise.
* testsuite/gas/mips/mips16-lst.d: Likewise.
* testsuite/gas/mips/micromips-lst.d: Likewise.
* testsuite/gas/mips/mips.exp: Add new tests.
---
gas/config/tc-mips.c | 8 +-------
gas/testsuite/gas/mips/lst.s | 5 +++++
gas/testsuite/gas/mips/micromips-lst.d | 16 ++++++++++++++++
gas/testsuite/gas/mips/mips.exp | 4 ++++
gas/testsuite/gas/mips/mips16-lst.d | 16 ++++++++++++++++
gas/testsuite/gas/mips/mips32-lst.d | 15 +++++++++++++++
6 files changed, 57 insertions(+), 7 deletions(-)
create mode 100644 gas/testsuite/gas/mips/lst.s
create mode 100644 gas/testsuite/gas/mips/micromips-lst.d
create mode 100644 gas/testsuite/gas/mips/mips16-lst.d
create mode 100644 gas/testsuite/gas/mips/mips32-lst.d
diff --git a/gas/config/tc-mips.c b/gas/config/tc-mips.c
index 4022829..2d57278 100644
--- a/gas/config/tc-mips.c
+++ b/gas/config/tc-mips.c
@@ -7486,13 +7486,7 @@ append_insn (struct mips_cl_insn *ip, expressionS *address_expr,
case APPEND_SWAP:
{
struct mips_cl_insn delay = history[0];
- if (mips_opts.mips16)
- {
- know (delay.frag == ip->frag);
- move_insn (ip, delay.frag, delay.where);
- move_insn (&delay, ip->frag, ip->where + insn_length (ip));
- }
- else if (relaxed_branch || delay.frag != ip->frag)
+ if (relaxed_branch || delay.frag != ip->frag)
{
/* Add the delay slot instruction to the end of the
current frag and shrink the fixed part of the
diff --git a/gas/testsuite/gas/mips/lst.s b/gas/testsuite/gas/mips/lst.s
new file mode 100644
index 0000000..e7853d7
--- /dev/null
+++ b/gas/testsuite/gas/mips/lst.s
@@ -0,0 +1,5 @@
+test:
+ move $16, $2
+ jr $31
+ addiu $16, $17, 0xff
+ jr $31
diff --git a/gas/testsuite/gas/mips/micromips-lst.d b/gas/testsuite/gas/mips/micromips-lst.d
new file mode 100644
index 0000000..270574e
--- /dev/null
+++ b/gas/testsuite/gas/mips/micromips-lst.d
@@ -0,0 +1,16 @@
+#objdump: -dr -M reg-names=numeric
+#as: -mips32r2 -32 -mmicromips -O2 -aln=lst.lst
+#name: microMIPS assembler listing file test
+#source: lst.s
+
+# Check delay slot filling with a listing file works
+
+.*: +file format .*mips.*
+
+Disassembly of section .text:
+0+ <.*>:
+ 0: 459f jr \$31
+ 2: 0e02 move \$16,\$2
+ 4: 459f jr \$31
+ 6: 3211 00ff addiu \$16,\$17,255
+ a: 0c00 nop
diff --git a/gas/testsuite/gas/mips/mips.exp b/gas/testsuite/gas/mips/mips.exp
index 4071434..c398dce 100644
--- a/gas/testsuite/gas/mips/mips.exp
+++ b/gas/testsuite/gas/mips/mips.exp
@@ -1253,6 +1253,10 @@ if { [istarget mips*-*-vxworks*] } {
run_dump_test "micromips-b16"
run_list_test "micromips-ill"
+ run_dump_test "mips32-lst"
+ run_dump_test "mips16-lst"
+ run_dump_test "micromips-lst"
+
run_dump_test_arches "mcu" [mips_arch_list_matching mips32r2 \
!octeon]
run_dump_test_arches "hilo-diff-eb" [mips_arch_list_all]
diff --git a/gas/testsuite/gas/mips/mips16-lst.d b/gas/testsuite/gas/mips/mips16-lst.d
new file mode 100644
index 0000000..cc68157
--- /dev/null
+++ b/gas/testsuite/gas/mips/mips16-lst.d
@@ -0,0 +1,16 @@
+#objdump: -dr -M reg-names=numeric
+#as: -mips32r2 -32 -mips16 -O2 -aln=lst.lst
+#name: MIPS16 assembler listing file test
+#source: lst.s
+
+# Check delay slot filling with a listing file works
+
+.*: +file format .*mips.*
+
+Disassembly of section .text:
+0+ <.*>:
+ 0: e820 jr \$31
+ 2: 6702 move \$16,\$2
+ 4: f0f0 410f addiu \$16,\$17,255
+ 8: e8a0 jrc \$31
+ a: 6500 nop
diff --git a/gas/testsuite/gas/mips/mips32-lst.d b/gas/testsuite/gas/mips/mips32-lst.d
new file mode 100644
index 0000000..1762d88
--- /dev/null
+++ b/gas/testsuite/gas/mips/mips32-lst.d
@@ -0,0 +1,15 @@
+#objdump: -dr -M reg-names=numeric
+#as: -mips32r2 -32 -O2 -aln=lst.lst
+#name: MIPS32 assembler listing file test
+#source: lst.s
+
+# Check delay slot filling with a listing file works
+
+.*: +file format .*mips.*
+
+Disassembly of section .text:
+0+ <.*>:
+ 0: 03e00008 jr \$31
+ 4: 00408025 move \$16,\$2
+ 8: 03e00008 jr \$31
+ c: 263000ff addiu \$16,\$17,255
--
2.2.1