| Message ID | 20140225110355.GI3386@bubble.grove.modra.org |
|---|---|
| State | New |
| Headers | show |
On Tue, Feb 25, 2014 at 02:30:59PM +0100, Ulrich Weigand wrote: > Alan Modra wrote: > > > Normally, reload 2, the secondary reload for reload 3, would result in > > a call to reload_v16qi_si_load with (reg:V16QI 78 1) as its "reg" > > argument, and the mem home for pseudo reg 159 as its "mem" arg. > > > > However, reload1.c:choose_reload_regs has code to, as the comment > > says: > > /* First see if this pseudo is already available as reloaded > > for a previous insn. > > > > In this case reload finds such a register, so reload_v16qi_si_load > > then sees the register as its "mem" argument, and spits the dummy. > > This still seems odd to me, I don't think the intent was that the > back-end secondary reload code is supposed to handle such cases. > > Instead, there's code in emit_input_reload_insns that is supposed > to re-check whether a secondary reload is still needed if something > changed significantly, e.g. if a value was inherited: > > /* If we have a secondary reload, pick up the secondary register > and icode, if any. If OLDEQUIV and OLD are different or > if this is an in-out reload, recompute whether or not we > still need a secondary register and what the icode should > be. If we still need a secondary register and the class or > icode is different, go back to reloading from OLD if using > OLDEQUIV means that we got the wrong type of register. We > cannot have different class or icode due to an in-out reload > because we don't make such reloads when both the input and > output need secondary reload registers. */ > > Note the condition "if OLDEQUIV and OLD are different" should be > true if the input value was inherited. Can you check whether > this case is hit with your test case? No. oldequiv and old are both (reg:V16QI 32 0). reload_override_in[j] is (reg:V16QI 32 0), and rl->in_reg is (subreg:V16QI (reg:V2DI 159 [ D.2446 ]) 0), so perhaps we're missing a subreg test in if (reload_override_in[j] && REG_P (rl->in_reg)) { oldequiv = old; old = rl->in_reg; } I'll follow this up after getting some sleep. Right now I'm too tired to think straight. Note that rs6000_secondary_reload_inner is not emitting a nop.. c4: 39 21 00 40 addi r9,r1,64 c8: 38 61 00 20 addi r3,r1,32 cc: 7c 3c ee 99 lxvd2x vs33,r28,r29 d0: 7c 1f ee 98 lxvd2x vs0,r31,r29 d4: 38 9f 00 d0 addi r4,r31,208 d8: 38 a0 00 10 li r5,16 dc: 7c 00 4f 98 stxvd2x vs0,0,r9 (1) e0: 10 01 ed c4 vsld v0,v1,v29 e4: f0 20 04 91 xxlor vs33,vs0,vs0 (2) e8: 10 00 07 ab vperm v0,v0,v0,v30 ec: 10 00 f8 00 vaddubm v0,v0,v31 f0: 10 01 08 2b vperm v0,v1,v1,v0 (3) (1) the previous reload of pseudo 159 (2) the insn emitted by rs6000_secondary_reload_inner (3) insn 61 (v1==vs33)
Index: gcc/config/rs6000/rs6000.c =================================================================== --- gcc/config/rs6000/rs6000.c (revision 208097) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -16363,7 +16363,18 @@ rs6000_secondary_reload_inner (rtx reg, rtx mem, r rs6000_secondary_reload_fail (__LINE__, reg, mem, scratch, store_p); if (GET_CODE (mem) != MEM) - rs6000_secondary_reload_fail (__LINE__, reg, mem, scratch, store_p); + { + /* If MEM was the home of a pseudo reg that wasn't allocated a + hard register, then when optimising reload will look at + previous instructions to see whether the MEM has already been + reloaded into a hard register. If that register is still + valid then we'll see it here rather than the MEM. */ + if (store_p) + emit_insn (gen_rtx_SET (VOIDmode, mem, reg)); + else + emit_insn (gen_rtx_SET (VOIDmode, reg, mem)); + return; + } rclass = REGNO_REG_CLASS (regno); addr = find_replacement (&XEXP (mem, 0));
PR57936 is regarding a reload problem with rs6000_secondary_reload_inner. This is the failing instruction, as at the start of reload: (insn 61 60 62 3 (set (reg:V16QI 220) (unspec:V16QI [ (subreg:V16QI (reg:V2DI 159 [ D.2446 ]) 0) (subreg:V16QI (reg:V2DI 159 [ D.2446 ]) 0) (reg:V16QI 213) ] UNSPEC_VPERM)) /home/pthaugen/src/gcc/gcc-4_9-power8/gcc/gcc/testsuite/g++.dg/torture/vshuf-main.inc:15 1253 {altivec_vperm_v16qi} (expr_list:REG_DEAD (reg:V16QI 213) (expr_list:REG_DEAD (reg:V2DI 159 [ D.2446 ]) (nil)))) You'll notice that none of the operands got a hard register. Here are the reloads: Reloads for insn # 61 Reload 0: BASE_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine reload_in_reg: (plus:SI (reg/f:SI 1 1) (const_int 64 [0x40])) Reload 1: BASE_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 2), can't combine reload_in_reg: (plus:SI (reg/f:SI 1 1) (const_int 64 [0x40])) Reload 2: BASE_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 1), can't combine, secondary_reload_p reload_reg_rtx: (reg:SI 10 10) Reload 3: reload_in (V16QI) = (reg:V16QI 32 0) ALTIVEC_REGS, RELOAD_FOR_INPUT (opnum = 1), can't combine reload_in_reg: (subreg:V16QI (reg:V2DI 159 [ D.2446 ]) 0) reload_reg_rtx: (reg:V16QI 78 1) secondary_in_reload = 2 secondary_in_icode = reload_v16qi_si_load Reload 4: BASE_REGS, RELOAD_FOR_INPUT_ADDRESS (opnum = 2), can't combine, secondary_reload_p reload_reg_rtx: (reg:SI 10 10) Reload 5: reload_in (V16QI) = (reg:V16QI 32 0) ALTIVEC_REGS, RELOAD_FOR_INPUT (opnum = 2), can't combine reload_in_reg: (subreg:V16QI (reg:V2DI 159 [ D.2446 ]) 0) reload_reg_rtx: (reg:V16QI 78 1) secondary_in_reload = 4 secondary_in_icode = reload_v16qi_si_load Reload 3 completely reloads pseudo reg 159, which lives at (mem/c:V16QI (plus:SI (reg/f:SI 1 1) (const_int 64 [0x40])) [4 %sfp+64 S16 A128]) That's good and proper (except that duplicate reload 5 does the same thing unnecessarily, which is cleaned up later BTW). Pseudos that don't get a hard reg must be reloaded to a reg to satisfy the altivec_vperm insn constraint of "v". Normally, reload 2, the secondary reload for reload 3, would result in a call to reload_v16qi_si_load with (reg:V16QI 78 1) as its "reg" argument, and the mem home for pseudo reg 159 as its "mem" arg. However, reload1.c:choose_reload_regs has code to, as the comment says: /* First see if this pseudo is already available as reloaded for a previous insn. In this case reload finds such a register, so reload_v16qi_si_load then sees the register as its "mem" argument, and spits the dummy. I don't think the generic reload machinery is doing anything wrong at this point, although it is a little unusual in that secondary reloads like reload 2 above are not deleted when inheriting reloads. (Other reloads are. See remove_address_replacements calls in choose_reload_regs.) I guess there may be some circumstances when the secondary reload can't just be replaced with an insn moving from one reg to another.. Bootstrapped and regression tested powerpc64-linux. OK to apply? PR target/57936 * config/rs6000/rs6000.c (rs6000_secondary_reload_inner): Do not fail when "mem" arg is not a MEM.