This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
Another reload inheritance bug
- From: Rask Ingemann Lambertsen <rask at sygehus dot dk>
- To: gcc at gcc dot gnu dot org
- Date: Mon, 11 Jun 2007 15:19:11 +0200
- Subject: Another reload inheritance bug
I've run into a case where reload clobbers a register which it had
decided to use for reload inheritance. Reload is given this:
(insn 623 430 431 35 ../../.././gcc/dp-bit.c:734 (set (reg:HI 162 [+6 ])
(mem/c/i:HI (plus:HI (reg/f:HI 10 bp)
(const_int -2 [0xfffffffe])) [0 S2 A16])) 9 {*movhi} (nil)
(expr_list:REG_EQUIV (mem/c/i:HI (plus:HI (reg/f:HI 10 bp)
(const_int -2 [0xfffffffe])) [0 S2 A16])
(nil)))
(insn 431 623 432 35 ../../.././gcc/dp-bit.c:734 (set (mem/s/j:HI (plus:HI (reg/v/f:HI 37 [ tmp ])
(const_int 10 [0xa])) [0 <variable>.fraction.ll+6 S2 A16])
(reg:HI 162 [+6 ])) 9 {*movhi} (insn_list:REG_DEP_TRUE 421 (nil))
(expr_list:REG_DEAD (reg:HI 162 [+6 ])
(nil)))
(note 432 431 433 35)
(note 433 432 434 35)
(insn 434 433 435 35 ../../.././gcc/dp-bit.c:735 (parallel [
(set (mem/s/j:HI (plus:HI (reg/v/f:HI 37 [ tmp ])
(const_int 2 [0x2])) [0 <variable>.normal_exp+0 S2 A16])
(plus:HI (mem/s/j:HI (plus:HI (reg/v/f:HI 37 [ tmp ])
(const_int 2 [0x2])) [0 <variable>.normal_exp+0 S2 A16])
(const_int 1 [0x1])))
(clobber (reg:CC 13 cc))
]) 44 {*addhi3} (nil)
(expr_list:REG_UNUSED (reg:CC 13 cc)
(nil)))
Reload faces two problems here:
1) Eliminating %bp to %sp where %sp can not be used as a base register.
2) Register 37 didn't get a hard reg, so it is on the stack.
Reload handles insns 623 and 431 fine and correctly records that register
37 has been loaded into %bx (reg:HI 6 b) as a result. Unfortunately, reload
clobbers %bx in insn 897 before using %bx in insn 434.
movw %sp, %bx ;# 895 *movhi/1
movw 74(%bx),%ax ;# 623 *movhi/1
movw 94(%bx),%bx ;# 896 *movhi/1
movw %ax, 10(%bx) ;# 431 *movhi/2
movw %sp, %bx ;# 897 *movhi/1
incw 2(%bx) ;# 434 *addhi3/1
Everything looks fine early in choose_reload_regs():
(gdb) call debug_rtx (chain->insn)
(insn 434 433 435 35 ../../.././gcc/dp-bit.c:735 (parallel [
(set (mem/s/j:HI (plus:HI (reg/v/f:HI 37 [ tmp ])
(const_int 2 [0x2])) [0 <variable>.normal_exp+0 S2 A16])
(plus:HI (mem/s/j:HI (plus:HI (reg/v/f:HI 37 [ tmp ])
(const_int 2 [0x2])) [0 <variable>.normal_exp+0 S2 A16])
(const_int 1 [0x1])))
(clobber (reg:CC 13 cc))
]) 44 {*addhi3} (nil)
(expr_list:REG_UNUSED (reg:CC 13 cc)
(nil)))
The *addhi3 pattern is nothing unusual. We're using the first
alternative of adding one to a memory operand:
(define_insn "*addhi3"
[(set (match_operand:HI 0 "nonimmediate_operand" "=rm,rm,qm,qm,r,m,!r,!r")
(plus:HI (match_operand:HI 1 "nonimmediate_operand" "%0,0,0,0,0,0,*w,*B")
(match_operand:HI 2 "general_operand" "P1,M1,Um,Uo,g,ri,*x,i")))
(clobber (reg:CC CC_REG))]
(gdb) call debug_reload()
Reload order: 0 1 2 3 4
Reload 0: reload_in (HI) = (reg/f:HI 12 sp)
BASE_REGS, RELOAD_FOR_OPADDR_ADDR (opnum = 0)
reload_in_reg: (reg/f:HI 12 sp)
Reload 1: reload_in (HI) = (mem/f/c/i:HI (plus:HI (reg/f:HI 12 sp)
(const_int 94 [0x5e])) [0 tmp+0 S2 A16])
BASE_REGS, RELOAD_FOR_OPERAND_ADDRESS (opnum = 0), can't combine
reload_in_reg: (reg/v/f:HI 37 [ tmp ])
Reload 2: BASE_REGS, RELOAD_FOR_OPERAND_ADDRESS (opnum = 0)
reload_in_reg: (reg/f:HI 12 sp)
Reload 3: BASE_REGS, RELOAD_FOR_OPERAND_ADDRESS (opnum = 0), can't combine
reload_in_reg: (reg/v/f:HI 37 [ tmp ])
Reload 4: reload_in (HI) = (mem/s/j:HI (plus:HI (reg/v/f:HI 37 [ tmp ])
(const_int 2 [0x2])) [0 <variable>.normal_exp+0 S2 A16])
reload_out (HI) = (mem/s/j:HI (plus:HI (reg/v/f:HI 37 [ tmp ])
(const_int 2 [0x2])) [0 <variable>.normal_exp+0 S2 A16])
GENERAL_REGS, RELOAD_OTHER (opnum = 0), optional
reload_in_reg: (mem/s/j:HI (plus:HI (reg/v/f:HI 37 [ tmp ])
(const_int 2 [0x2])) [0 <variable>.normal_exp+0 S2 A16])
reload_out_reg: (mem/s/j:HI (plus:HI (reg/v/f:HI 37 [ tmp ])
(const_int 2 [0x2])) [0 <variable>.normal_exp+0 S2 A16])
reload 0:
(gdb) print regno
$61 = 12
(gdb) call debug_rtx(reg_last_reload_reg[regno])
(reg:HI 6 b)
(gdb) print /x reg_reloaded_valid
$67 = 0xc0
(gdb) print reg_reloaded_contents[6]
$64 = 37
(gdb) print reg_reloaded_contents[7]
$65 = 37
(gdb) print reload_inherited[0]@5
$70 = "\000\000\000\000"
I.e. reload looks at register 6 for reload inheritance, but the contents
aren't right, so no reload inheritance for reload 0.
reload 1:
(gdb) print regno
$77 = 37
(gdb) print mode
$78 = HImode
(gdb) call debug_rtx(reg_last_reload_reg[regno])
(reg:HI 6 b)
(gdb) s
free_for_value_p (regno=6, mode=HImode, opnum=0, type=RELOAD_FOR_OPERAND_ADDRESS, value=0xb7d58f18, out=0xb7e72210, reloadnum=1, ignore_address_reloads=1)
at ../../../cvssrc/gcc/gcc/reload1.c:5294
(gdb) fin
Run till exit from #0 free_for_value_p (regno=6, mode=HImode, opnum=0, type=RELOAD_FOR_OPERAND_ADDRESS, value=0xb7d58f18, out=0xb7e72210, reloadnum=1,
ignore_address_reloads=1) at ../../../cvssrc/gcc/gcc/reload1.c:5300
0x083cd89e in choose_reload_regs (chain=0x90e4fe8) at ../../../cvssrc/gcc/gcc/reload1.c:5792
Value returned is $85 = 1
So reload decides to use hard reg 6 (%bx) for reg 37 because the previous
reload can be inherited:
(gdb) s
mark_reload_reg_in_use (regno=6, opnum=0, type=RELOAD_FOR_OPERAND_ADDRESS, mode=HImode) at ../../../cvssrc/gcc/gcc/reload1.c:4449
(gdb) print /x reload_reg_used_in_op_addr
$90 = 0xc0
(gdb) print /x reload_reg_used_at_all
$91 = 0xc0
Additionally, it is recorded that this reload uses inheritance:
rld[r].reg_rtx = last_reg;
reload_inherited[r] = 1;
reload_inheritance_insn[r]
= reg_reloaded_insn[i];
reload_spill_index[r] = i;
for (k = 0; k < nr; k++)
SET_HARD_REG_BIT (reload_reg_used_for_inherit,
i + k);
For reload 0, choose_reload_regs() needs to allocate a register:
(gdb) s
allocate_reload_reg (chain=0x90e4fe8, r=0, last_reload=0) at ../../../cvssrc/gcc/gcc/reload1.c:5439
We have
(gdb) print /x reload_reg_used_in_op_addr_reload
$102 = 0x0
(gdb) print /x reload_reg_used_at_all
$110 = 0xc0
(gdb) print /x reload_reg_used_for_inherit
$111 = 0xc0
allocate_reload_reg() checks reload_reg_free_p(), where we have:
case RELOAD_FOR_OPADDR_ADDR:
for (i = 0; i < reload_n_operands; i++)
if (TEST_HARD_REG_BIT (reload_reg_used_in_input[i], regno))
return 0;
return (!TEST_HARD_REG_BIT (reload_reg_used_in_op_addr_reload, regno));
but no check for reload_reg_used_for_inherit. In fact, the only place where
reload_reg_used_for_inherit is checked is in allocate_reload_reg()
/* Look first for regs to share, then for unshared. But
don't share regs used for inherited reloads; they are
the ones we want to preserve. */
&& (pass
|| (TEST_HARD_REG_BIT (reload_reg_used_at_all,
regnum)
&& ! TEST_HARD_REG_BIT (reload_reg_used_for_inherit,
regnum))))
and I'm wondering if perhaps the parentheses have become misplaced. I
mean, the code would better match the comment if written as
&& (pass
|| TEST_HARD_REG_BIT (reload_reg_used_at_all,
regnum))
&& ! TEST_HARD_REG_BIT (reload_reg_used_for_inherit,
regnum)))
but the code seems to match the log entry:
----------
r3991 | kenner | 1993-04-03 01:43:16 +0200 (l, 03 apr 1993) | 8 lines
(reload_reg_used_for_inherit): New variable.
(clear_reload_reg_in_use): New function.
(allocate_reload_reg): Don't consider an inherited register as one that we
should share in the first pass.
Don't mark a register in use until we are sure it will fit.
(choose_reload_regs): Mark spill regs used for inheriting.
When we decide we can no longer use a register, show it isn't being used.
----------
I.e. we're deliberately clobbering inherited registers the second time
around.
So when the time comes to actually emit the reload insns, we emit insn
897 for reload 0, clobbering %bx (reg:HI 6 b):
(gdb) fin
Run till exit from #0 gen_reload (out=0xb7d7e460, in=0xb7ea6000, opnum=0, type=RELOAD_FOR_OPADDR_ADDR) at ../../../cvssrc/gcc/gcc/reload1.c:8047
emit_input_reload_insns (chain=0x99b1000, rl=0x88ed780, old=0xb7ea6000, j=0) at ../../../cvssrc/gcc/gcc/reload1.c:6978
Value returned is $10 = (struct rtx_def *) 0xb7d8c3e8
(gdb) call debug_rtx($10)
(insn 897 0 0 (set (reg:HI 6 b)
(reg/f:HI 12 sp)) -1 (nil)
(nil))
On to reload 1:
(gdb) s
do_input_reload (chain=0x99b1000, rl=0x88ed7cc, j=1) at ../../../cvssrc/gcc/gcc/reload1.c:7221
(gdb) print *rl
$16 = {in = 0xb7d8bf18, out = 0x0, class = BASE_REGS, inmode = HImode, outmode = VOIDmode, mode = HImode, nregs = 4, inc = 0, in_reg = 0xb7f98e20,
out_reg = 0x0, regno = -1, reg_rtx = 0xb7d7e460, opnum = 0, secondary_in_reload = -1, secondary_out_reload = -1, secondary_in_icode = CODE_FOR_nothing,
secondary_out_icode = CODE_FOR_nothing, when_needed = RELOAD_FOR_OPERAND_ADDRESS, optional = 0, nocombine = 1, secondary_p = 0, nongroup = 0}
(gdb) print reload_inherited[0]@5
$19 = "\000\001\000\000"
Because reload_inherited[1] = 1, do_input_reload() doesn't call
emit_input_reload_insns(). There is no check that the reload register still
contains the correct value, and besides, reg_reloaded_valid and
reg_reloaded_contents aren't updated until later anyway.
--
Rask Ingemann Lambertsen