This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] MIPS16, delay slot filling, assembly listing files


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]