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]

[committed] PR gas/12915: MIPS prev_nop_frag assert


PR gas/12915 is caused by a bug in the code that handles hazards
between a ".set reorder" block and a following ".set noreorder" block.
In general, these hazards can apply for up to MAX_NOPS (4) instructions
into the latter block.

We record a conservative worst case in prev_nop_frag_holds as soon as
we end the ".set reorder" block, and insert that many nops at the end
of its (variable) frag.  Then, for up to the first prev_nop_frag_holds
instructions in the ".set noreorder" block, we check how many nops are
actually needed.  E.g. for prev_nop_frag_holds == 4:

    INSN1         1-+
    INSN2         2-+  1-+
    INSN3         3-+  2-+  1-+
    INSN4         4-+  3-+  2-+  1-+
    .set noreorder  |    |    |    |
    INSN5         5-+    |    |    |
    INSN6              5-+    |    |
    INSN7                   5-+    |
    INSN8                        5-+

where the numbers on the right hand side indicate relative instruction
positions.

The problem is that this check also includes any hazards generated by
the previous instructions in the ".set noreorder" block too, even though
they aren't relevant here; the user is asserting that there are no such
hazards.  That is, we had:

    INSN1         1-+
    INSN2         2-+  1-+
    INSN3         3-+  2-+  1-+
    INSN4         4-+  3-+  2-+  1-+
    .set noreorder  |    |    |    |
    INSN5         5-+  4-+  3-+  2-+
    INSN6              5-+  4-+  3-+
    INSN7                   5-+  4-+
    INSN8                        5-+

Which is a pretty basic oversight (of mine) really :-(

The patch below restricts the check to instructions outside the current
".set noreorder" block.  Tested on mips64-linux-gnu and applied.

Note that both earlier and now, I deliberately avoided using a circular
buffer (or other such structure) to avoid complete writes to the array.
The array is so small that it seemed like overkill, especially given
some of the history manipulations we need to do.

Richard


gas/
	PR gas/12915
	* config/tc-mips.c (append_insn): Only consider hazards between the
	pre-noreorder block and ip.

gas/testsuite/
	* gas/mips/pr12915.s, gas/mips/pr12915.d: New test.
	* gas/mips/mips.exp: Run it.

Index: gas/config/tc-mips.c
===================================================================
--- gas/config/tc-mips.c	2011-06-22 20:49:06.000000000 +0100
+++ gas/config/tc-mips.c	2011-06-22 20:49:06.000000000 +0100
@@ -3154,8 +3154,18 @@ append_insn (struct mips_cl_insn *ip, ex
     }
   else if (mips_relax.sequence != 2 && prev_nop_frag != NULL)
     {
-      /* Work out how many nops in prev_nop_frag are needed by IP.  */
-      int nops = nops_for_insn_or_target (history, ip);
+      struct mips_cl_insn stubbed_history[ARRAY_SIZE (history)];
+      int nops, i;
+
+      /* Work out how many nops in prev_nop_frag are needed by IP.
+	 Base this on a history in which all insns since prev_nop_frag
+	 are stubbed out with nops.  */
+      for (i = 0; i < (int) ARRAY_SIZE (history); i++)
+	if (i < prev_nop_frag_since)
+	  stubbed_history[i] = *NOP_INSN;
+	else
+	  stubbed_history[i] = history[i];
+      nops = nops_for_insn_or_target (stubbed_history, ip);
       gas_assert (nops <= prev_nop_frag_holds);
 
       /* Enforce NOPS as a minimum.  */
Index: gas/testsuite/gas/mips/pr12915.s
===================================================================
--- /dev/null	2011-06-22 20:57:30.053243972 +0100
+++ gas/testsuite/gas/mips/pr12915.s	2011-06-22 20:54:17.000000000 +0100
@@ -0,0 +1,5 @@
+	lui	$27, %hi(kernelsp)
+	lw	$27, %lo(kernelsp)($27)
+	.set	noreorder
+	mfc0	$28, $14
+	addu	$28, 4
Index: gas/testsuite/gas/mips/pr12915.d
===================================================================
--- /dev/null	2011-06-22 20:57:30.053243972 +0100
+++ gas/testsuite/gas/mips/pr12915.d	2011-06-22 20:56:44.000000000 +0100
@@ -0,0 +1,15 @@
+#as: -32 -mips1
+#objdump: -dr
+
+.*
+
+
+Disassembly of section \.text:
+
+00000000 <\.text>:
+   0:	3c1b0000 	lui	k1,0x0
+			0: R_MIPS_HI16	kernelsp
+   4:	8f7b0000 	lw	k1,0\(k1\)
+			4: R_MIPS_LO16	kernelsp
+   8:	401c7000 	mfc0	gp,c0_epc
+   c:	279c0004 	addiu	gp,gp,4
Index: gas/testsuite/gas/mips/mips.exp
===================================================================
--- gas/testsuite/gas/mips/mips.exp	2011-06-22 20:52:54.000000000 +0100
+++ gas/testsuite/gas/mips/mips.exp	2011-06-22 20:52:56.000000000 +0100
@@ -1000,4 +1000,6 @@ if { [istarget mips*-*-vxworks*] } {
 					[mips_arch_list_matching mips32r2] ] ]
 
     if $has_newabi { run_dump_test "cfi-n64-1" }
+
+    run_dump_test "pr12915"
 }


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