This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: fix opt/2960
- From: "Ulrich Weigand" <Ulrich dot Weigand at de dot ibm dot com>
- To: Richard Henderson <rth at redhat dot com>
- Cc: dje at watson dot ibm dot com, gcc-patches at gcc dot gnu dot org
- Date: Mon, 7 Oct 2002 21:10:28 +0200
- Subject: Re: fix opt/2960
- Sensitivity:
Richard Henderson wrote:
>If you can point to which instructions get incorrectly scheduled,
>it should be fairly easy for me to track this down...
OK, I've looked at 20010910-1.c on s390. The code in question is this:
static void epic_init_ring(struct epic_private *ep)
{
int i;
for (i = 0; i < 5; i++)
{
ep->rx_ring[i].next = 10 + (i+1)*2;
ep->rx_skbuff[i] = 0;
}
ep->rx_ring[i-1].next = 10;
}
The statement after the loop gets miscompiled; it doesn't set rx_ring[4],
but instead rx_ring[3].
When compiled with -Os, we get this insn stream before sched (20.regmove):
;; Start of basic block 0, registers live: 2 [%r2] 11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp]
(note 57 2 3 0 [bb 0] NOTE_INSN_BASIC_BLOCK)
(insn 3 57 4 0 (nil) (set (reg/v/f:SI 40)
(reg:SI 2 %r2)) 52 {*movsi} (nil)
(expr_list:REG_DEAD (reg:SI 2 %r2)
(nil)))
(note 4 3 10 0 NOTE_INSN_FUNCTION_BEG)
(insn 10 4 96 0 0x40186900 (set (reg/v:SI 41)
(const_int 0 [0x0])) 50 {*movsi_lhi} (nil)
(expr_list:REG_EQUAL (const_int 0 [0x0])
(nil)))
(insn 96 10 104 0 (nil) (set (reg:SI 63)
(const_int 12 [0xc])) 50 {*movsi_lhi} (nil)
(expr_list:REG_EQUAL (const_int 12 [0xc])
(nil)))
(insn 104 96 11 0 (nil) (set (reg:SI 68)
(const_int 5 [0x5])) 50 {*movsi_lhi} (nil)
(expr_list:REG_EQUAL (const_int 5 [0x5])
(nil)))
(note 11 104 40 0 NOTE_INSN_LOOP_BEG)
(jump_insn 40 11 41 0 0x40186900 (set (pc)
(label_ref 12)) 261 {jump} (nil)
(nil))
;; End of basic block 0, registers live:
11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp] 40 41 63 68
(barrier 41 40 103)
;; Start of basic block 1, registers live: 11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp] 40 41 57 58 63 68
(code_label 103 41 58 1 7 "" [1 uses])
(note 58 103 29 1 [bb 1] NOTE_INSN_BASIC_BLOCK)
(insn 29 58 34 1 0x40186900 (set (mem/s:SI (plus:SI (reg:SI 58)
(reg:SI 57)) [7 <variable>.next+0 S4 A32])
(reg:SI 63)) 52 {*movsi} (nil)
(expr_list:REG_DEAD (reg:SI 57)
(nil)))
(insn 34 29 36 1 0x40186900 (set (mem/s:SI (plus:SI (plus:SI (reg:SI 58)
(reg/v/f:SI 40))
(const_int 4 [0x4])) [7 <variable>.rx_skbuff S4 A32])
(const_int 0 [0x0])) 52 {*movsi} (nil)
(expr_list:REG_DEAD (reg:SI 58)
(nil)))
(note 36 34 38 1 NOTE_INSN_LOOP_CONT)
(insn 38 36 95 1 0x40186900 (parallel [
(set (reg/v:SI 41)
(plus:SI (reg/v:SI 41)
(const_int 1 [0x1])))
(clobber (reg:CC 33 %cc))
]) 131 {addsi3} (nil)
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil)))
(insn 95 38 12 1 (nil) (parallel [
(set (reg:SI 63)
(plus:SI (reg:SI 63)
(const_int 2 [0x2])))
(clobber (reg:CC 33 %cc))
]) 131 {addsi3} (nil)
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil)))
;; End of basic block 1, registers live:
11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp] 40 41 63 68
;; Start of basic block 2, registers live: 11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp] 40 41 63 68
(code_label 12 95 60 2 2 "" [1 uses])
(note 60 12 83 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(insn 83 60 86 2 (nil) (set (reg:SI 57)
(mem/s:SI (reg/v/f:SI 40) [5 <variable>.rx_ring+0 S4 A32])) 52 {*movsi} (nil)
(nil))
(insn 86 83 102 2 (nil) (set (reg:SI 58)
(ashift:SI (reg/v:SI 41)
(const_int 2 [0x2]))) 244 {ashlsi3} (nil)
(nil))
(jump_insn 102 86 44 2 (nil) (parallel [
(set (pc)
(if_then_else (ne (reg:SI 68)
(const_int 1 [0x1]))
(label_ref 103)
(pc)))
(set (reg:SI 68)
(plus:SI (reg:SI 68)
(const_int -1 [0xffffffffffffffff])))
(clobber (scratch:SI))
(clobber (reg:CC 33 %cc))
]) 257 {doloop_si} (nil)
(expr_list:REG_UNUSED (scratch:SI)
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(expr_list:REG_BR_PROB (const_int 8000 [0x1f40])
(nil)))))
;; End of basic block 2, registers live:
11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp] 40 41 57 58 63 68
(note 44 102 63 NOTE_INSN_LOOP_END)
;; Start of basic block 3, registers live: 11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp] 57 58
(note 63 44 48 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
(insn 48 63 51 3 0x40186900 (set (reg:SI 53)
(const_int -4096 [0xfffffffffffff000])) 50 {*movsi_lhi} (nil)
(expr_list:REG_EQUAL (const_int -4096 [0xfffffffffffff000])
(nil)))
(insn 51 48 52 3 0x40186900 (parallel [
(set (reg:SI 58)
(plus:SI (reg:SI 58)
(reg:SI 57)))
(clobber (reg:CC 33 %cc))
]) 131 {addsi3} (nil)
(expr_list:REG_DEAD (reg:SI 57)
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil))))
(insn 52 51 54 3 0x40186900 (set (mem/s:SI (plus:SI (plus:SI (reg:SI 58)
(reg:SI 53))
(const_int 4092 [0xffc])) [7 <variable>.next+0 S4 A32])
(const_int 10 [0xa])) 52 {*movsi} (insn_list 48 (insn_list 51 (nil)))
(expr_list:REG_DEAD (reg:SI 58)
(expr_list:REG_DEAD (reg:SI 53)
(nil))))
;; End of basic block 3, registers live:
11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp]
Note that we need reg 58 to contain the (scaled) index after the loop.
This is provided by insn 86. Obviously, insn 86 depends on insn 38
which increments reg 41.
However, after the scheduler, insn 86 has been moved before insn 38.
Note that insn 86 has no dependency on 38 according to the scheduler:
;; Start of basic block 0, registers live: 2 [%r2] 11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp]
(note 57 2 4 0 [bb 0] NOTE_INSN_BASIC_BLOCK)
(note 4 57 3 0 NOTE_INSN_FUNCTION_BEG)
(insn 3 4 10 0 (nil) (set (reg/v/f:SI 40)
(reg:SI 2 %r2)) 52 {*movsi} (nil)
(expr_list:REG_DEAD (reg:SI 2 %r2)
(nil)))
(insn 10 3 96 0 0x40186900 (set (reg/v:SI 41)
(const_int 0 [0x0])) 50 {*movsi_lhi} (nil)
(expr_list:REG_EQUAL (const_int 0 [0x0])
(nil)))
(insn 96 10 104 0 (nil) (set (reg:SI 63)
(const_int 12 [0xc])) 50 {*movsi_lhi} (nil)
(expr_list:REG_EQUAL (const_int 12 [0xc])
(nil)))
(insn 104 96 106 0 (nil) (set (reg:SI 68)
(const_int 5 [0x5])) 50 {*movsi_lhi} (nil)
(expr_list:REG_EQUAL (const_int 5 [0x5])
(nil)))
(note 106 104 40 0 NOTE_INSN_LOOP_BEG)
(jump_insn 40 106 41 0 0x40186900 (set (pc)
(label_ref 12)) 261 {jump} (insn_list 104 (insn_list 96 (insn_list 10 (insn_list 3 (nil)))))
(nil))
;; End of basic block 0, registers live:
11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp] 40 41 63 68
(barrier 41 40 103)
;; Start of basic block 1, registers live: 11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp] 40 41 57 58 63 68
(code_label 103 41 58 1 7 "" [1 uses])
(note 58 103 36 1 [bb 1] NOTE_INSN_BASIC_BLOCK)
(note 36 58 29 1 NOTE_INSN_LOOP_CONT)
(insn 29 36 95 1 0x40186900 (set (mem/s:SI (plus:SI (reg:SI 58)
(reg:SI 57)) [7 <variable>.next+0 S4 A32])
(reg:SI 63)) 52 {*movsi} (insn_list 86 (insn_list 83 (insn_list:REG_DEP_ANTI 102 (nil))))
(expr_list:REG_DEAD (reg:SI 57)
(nil)))
(insn 95 29 34 1 (nil) (parallel [
(set (reg:SI 63)
(plus:SI (reg:SI 63)
(const_int 2 [0x2])))
(clobber (reg:CC 33 %cc))
]) 131 {addsi3} (insn_list:REG_DEP_ANTI 29 (nil))
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil)))
(insn 34 95 12 1 0x40186900 (set (mem/s:SI (plus:SI (plus:SI (reg:SI 58)
(reg/v/f:SI 40))
(const_int 4 [0x4])) [7 <variable>.rx_skbuff S4 A32])
(const_int 0 [0x0])) 52 {*movsi} (insn_list 86 (insn_list:REG_DEP_ANTI 102 (insn_list:REG_DEP_OUTPUT 29 (nil))))
(expr_list:REG_DEAD (reg:SI 58)
(nil)))
;; End of basic block 1, registers live:
11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp] 40 41 63 68
;; Start of basic block 2, registers live: 11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp] 40 41 63 68
(code_label 12 34 60 2 2 "" [1 uses])
(note 60 12 86 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(insn 86 60 38 2 (nil) (set (reg:SI 58)
(ashift:SI (reg/v:SI 41)
(const_int 2 [0x2]))) 244 {ashlsi3} (nil)
(nil))
(insn 38 86 83 2 0x40186900 (parallel [
(set (reg/v:SI 41)
(plus:SI (reg/v:SI 41)
(const_int 1 [0x1])))
(clobber (reg:CC 33 %cc))
]) 131 {addsi3} (insn_list:REG_DEP_ANTI 86 (nil))
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil)))
(insn 83 38 102 2 (nil) (set (reg:SI 57)
(mem/s:SI (reg/v/f:SI 40) [5 <variable>.rx_ring+0 S4 A32])) 52 {*movsi} (nil)
(nil))
(jump_insn 102 83 44 2 (nil) (parallel [
(set (pc)
(if_then_else (ne (reg:SI 68)
(const_int 1 [0x1]))
(label_ref 103)
(pc)))
(set (reg:SI 68)
(plus:SI (reg:SI 68)
(const_int -1 [0xffffffffffffffff])))
(clobber (scratch:SI))
(clobber (reg:CC 33 %cc))
]) 257 {doloop_si} (insn_list:REG_DEP_ANTI 83 (insn_list:REG_DEP_ANTI 86 (nil)))
(expr_list:REG_UNUSED (scratch:SI)
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(expr_list:REG_BR_PROB (const_int 8000 [0x1f40])
(nil)))))
;; End of basic block 2, registers live:
11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp] 40 41 57 58 63 68
(note 44 102 63 NOTE_INSN_LOOP_END)
;; Start of basic block 3, registers live: 11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp] 57 58
(note 63 44 51 3 [bb 3] NOTE_INSN_BASIC_BLOCK)
(insn 51 63 48 3 0x40186900 (parallel [
(set (reg:SI 58)
(plus:SI (reg:SI 58)
(reg:SI 57)))
(clobber (reg:CC 33 %cc))
]) 131 {addsi3} (nil)
(expr_list:REG_DEAD (reg:SI 57)
(expr_list:REG_UNUSED (reg:CC 33 %cc)
(nil))))
(insn 48 51 52 3 0x40186900 (set (reg:SI 53)
(const_int -4096 [0xfffffffffffff000])) 50 {*movsi_lhi} (nil)
(expr_list:REG_EQUAL (const_int -4096 [0xfffffffffffff000])
(nil)))
(insn 52 48 105 3 0x40186900 (set (mem/s:SI (plus:SI (plus:SI (reg:SI 58)
(reg:SI 53))
(const_int 4092 [0xffc])) [7 <variable>.next+0 S4 A32])
(const_int 10 [0xa])) 52 {*movsi} (insn_list 51 (insn_list 48 (nil)))
(expr_list:REG_DEAD (reg:SI 58)
(expr_list:REG_DEAD (reg:SI 53)
(nil))))
;; End of basic block 3, registers live:
11 [%r11] 15 [%r15] 32 [%ap] 34 [%fp]
It appears to me that the jump (insn 40) from LOOP_BEG into the middle of
the loop confuses the scheduler. It starts its analysis of the dependencies
with basic block 2, and doesn't see any dependencies of insn 86 because
it hasn't actually scanned insn 38 yet and thus doesn't have reg 41 in
the reg_pending_sets set ...
Mit freundlichen Gruessen / Best Regards
Ulrich Weigand
--
Dr. Ulrich Weigand
Linux for S/390 Design & Development
IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
Phone: +49-7031/16-3727 --- Email: Ulrich.Weigand@de.ibm.com