Hi Paul, Matthew,
Paul -- as I'm preparing the R5900 kernel patches there was a USB DMA series
and breakage that needed attention. The fixes ending with ff2437befd8f ("usb:
host: Fix excessive alignment restriction for local memory allocations") are
now merged with Linus' kernel, and recommended for the R5900 series. The
initial R5900 patch submission is getting closer. :)
The present problem is related to GCC and the R5900 short loop bug[1]. It
turns out GCC emits assembly code like
loop: addiu $5,$5,1
addiu $4,$4,1
lb $2,-1($5)
.set noreorder
.set nomacro
bne $2,$3,loop
sb $2,-1($4)
.set macro
.set reorder
that is undefined for the R5900 (this short loop has five instructions),
for simple C code such as
while ((*s++ = *p++) != '\n')
;
in the kernel. The noreorder directive prohibits GAS from corrections, and
GAS really ought to give an error for it, I think. In the meantime, I have a
tool that does machine code analysis of ELF objects to catch undefined R5900
short loops, including those made with assembler macros in the kernel.
[ In theory, GAS could actually insert NOPs prior to the noreorder directive,
to make the loop longer that six instructions, but GAS does not have that
kind of capability. Another option that GCC prevents is to place a NOP in
the delay slot. ]
A reasonable fix for GCC is perhaps to update gcc/config/mips/mips.md to not
make explicit use of the branch delay slot, as suggested by the patch below?
Then GCC will emit
loop: addiu $5,$5,1
addiu $4,$4,1
lb $2,-1($5)
sb $2,-1($4)
bne $2,$3,loop
that GAS will adjust in the ELF object to
4: 24a50001 addiu a1,a1,1
8: 24840001 addiu a0,a0,1
c: 80a2ffff lb v0,-1(a1)
10: a082ffff sb v0,-1(a0)
14: 1443fffb bne v0,v1,4
18: 00000000 nop
where a NOP is placed in the delay slot to avoid the bug. For longer loops,
this relies on GAS making appropriate use of the delay slot. I'm not sure if
GAS is good at that, though?
Fredrik
References:
[1]
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gas/config/tc-mips.c;h=b7b4b6989a12d3091a02de7155fbea3adbf1c9d7;hb=HEAD#l7133
On the R5900 short loops need to be fixed by inserting a NOP in the
branch delay slot.
The short loop bug under certain conditions causes loops to execute
only once or twice. We must ensure that the assembler never
generates loops that satisfy all of the following conditions:
- a loop consists of less than or equal to six instructions
(including the branch delay slot);
- a loop contains only one conditional branch instruction at the end
of the loop;
- a loop does not contain any other branch or jump instructions;
- a branch delay slot of the loop is not NOP (EE 2.9 or later).
We need to do this because of a hardware bug in the R5900 chip.
diff --git a/gcc/config/mips/mips.md b/gcc/config/mips/mips.md
index e17b1d522f0..acd31a8960c 100644
--- a/gcc/config/mips/mips.md
+++ b/gcc/config/mips/mips.md
@@ -1091,6 +1091,7 @@
(define_delay (and (eq_attr "type" "branch")
(not (match_test "TARGET_MIPS16"))
+ (not (match_test "TARGET_FIX_R5900"))
(eq_attr "branch_likely" "yes"))
[(eq_attr "can_delay" "yes")
(nil)
@@ -1100,6 +1101,7 @@
;; not annul on false.
(define_delay (and (eq_attr "type" "branch")
(not (match_test "TARGET_MIPS16"))
+ (not (match_test "TARGET_FIX_R5900"))
(ior (match_test "TARGET_CB_NEVER")
(and (eq_attr "compact_form" "maybe")
(not (match_test "TARGET_CB_ALWAYS")))
