This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[committed] PR gas/12915: MIPS prev_nop_frag assert
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: binutils at sourceware dot org
- Date: Thu, 23 Jun 2011 21:24:00 +0100
- Subject: [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"
}