[RFC] Fix PR rtl-optimization/33732
Kenneth Zadeck
zadeck@naturalbridge.com
Wed Nov 7 22:29:00 GMT 2007
Rask Ingemann Lambertsen wrote:
> On Tue, Nov 06, 2007 at 02:19:16PM +0100, Rask Ingemann Lambertsen wrote:
>
>> Currently, we stop df from keeping its data up-to-date shortly before
>> calling reload() (in two places in global.c). Suppose we move that into
>> reload1.c/reload() itself after this bit:
>>
>> /* Alter each pseudo-reg rtx to contain its hard reg number.
>> Assign stack slots to the pseudos that lack hard regs or equivalents.
>> Do not touch virtual registers. */
>>
>> for (i = LAST_VIRTUAL_REGISTER + 1; i < max_regno; i++)
>> alter_reg (i, -1);
>>
>> At this point, df should be able to update the REG_DEAD notes. Then we
>> can turn off df rescanning for the rest of reload.
>>
>
> Dataflow maintainers, I'm going to ask for some advice here. Out of
>
> (insn 73 72 70 4 longlong.c:19 (set (reg:SI 21 %r21 [orig:154+4 ] [154])
> (lshiftrt:SI (reg:SI 28 %r28 [141])
> (subreg:SI (reg:DI 23 %r23 [116]) 4))) 277 {lshrsi3} (nil))
>
> (insn 70 73 71 4 longlong.c:19 (set (reg:SI 88 SAR [140])
> (minus:SI (const_int 31 [0x1f])
> (subreg:SI (reg:DI 21 %r21 [137]) 4))) 184 {*pa.md:5698} (nil))
>
> (insn 71 70 74 4 longlong.c:19 (set (reg:SI 19 %r19 [139])
> (ashift:SI (reg:SI 19 %r19 [135])
> (minus:SI (const_int 31 [0x1f])
> (reg:SI 88 SAR [140])))) 264 {zvdep32} (expr_list:REG_EQUAL (ashift:SI (reg:SI 19 %r19 [135])
> (subreg:SI (reg:DI 21 %r21 [137]) 4))
> (nil)))
>
> (insn 74 71 77 4 longlong.c:19 (set (reg:SI 21 %r21 [orig:154+4 ] [154])
> (ior:SI (reg:SI 19 %r19 [139])
> (reg:SI 21 %r21 [orig:154+4 ] [154]))) 210 {*pa.md:6217} (nil))
>
> creates this (notice what happens at insn 70):
>
> live at bottom 3 [%r3] 20 [%r20] 21 [%r21] 25 [%r25] 30 [%r30] 31 [%r31]
> live before artificials out 3 [%r3] 20 [%r20] 21 [%r21] 25 [%r25] 30 [%r30] 31 [%r31]
> regular looking at def d-1 reg 21 bb 4 insn 74 flag 0x8 type 0x0 loc b7f136b4(b7f152a0) chain { }
> regular looking at use u-1 reg 19 bb 4 insn 74 flag 0x8 type 0x1 loc b7f1369c(b7f13510) chain { }
> adding 4: 74 (expr_list:REG_DEAD (reg:SI 19 %r19 [139])
> (nil))
> regular looking at use u-1 reg 21 bb 4 insn 74 flag 0x8 type 0x1 loc b7f136a0(b7f152a0) chain { }
> regular looking at def d-1 reg 19 bb 4 insn 71 flag 0x8 type 0x0 loc b7f135c4(b7f13510) chain { }
> regular looking at use u-1 reg 19 bb 4 insn 71 flag 0x8 type 0x1 loc b7f135ac(b7f13420) chain { }
> regular looking at use u-1 reg 88 bb 4 insn 71 flag 0x8 type 0x1 loc b7f13598(b7f13540) chain { }
> adding 4: 71 (expr_list:REG_DEAD (reg:SI 88 SAR [140])
> (expr_list:REG_EQUAL (ashift:SI (reg:SI 19 %r19 [135])
> (subreg:SI (reg:DI 21 %r21 [137]) 4))
> (nil)))
> regular looking at def d-1 reg 88 bb 4 insn 70 flag 0x8 type 0x0 loc b7f1357c(b7f13540) chain { }
> regular looking at use u-1 reg 22 bb 4 insn 70 flag 0x18 type 0x1 loc b7f13568(b7f13528) chain { }
> adding 4: 70 (expr_list:REG_DEAD (reg:DI 21 %r21 [137])
> (nil))
> regular looking at def d-1 reg 21 bb 4 insn 73 flag 0x8 type 0x0 loc b7f13684(b7f152a0) chain { }
> regular looking at use u-1 reg 24 bb 4 insn 73 flag 0x18 type 0x1 loc b7f13670(b7f13618) chain { }
> adding 4: 73 (expr_list:REG_DEAD (reg:DI 23 %r23 [116])
> (nil))
> regular looking at use u-1 reg 28 bb 4 insn 73 flag 0x8 type 0x1 loc b7f1366c(b7f13630) chain { }
> adding 4: 73 (expr_list:REG_DEAD (reg:SI 28 %r28 [141])
> (expr_list:REG_DEAD (reg:DI 23 %r23 [116])
> (nil)))
>
> We get a REG_DEAD note covering both (reg:SI 21) and (reg:SI 22) despite
> the use of (reg:SI 21) in insn 74. As a result, reload thinks it can use
> (reg:SI 21) as reload register in insn 70:
>
> Reloads for insn # 73
> Reload 0: reload_in (SI) = (reg:SI 24 %r24)
> SHIFT_REGS, RELOAD_FOR_INPUT (opnum = 2), can't combine
> reload_in_reg: (subreg:SI (reg:DI 23 %r23 [116]) 4)
> reload_reg_rtx: (reg:SI 88 SAR)
>
> Reloads for insn # 70
> Reload 0: reload_out (SI) = (reg:SI 88 SAR [140])
> GENERAL_REGS, RELOAD_FOR_OUTPUT (opnum = 0)
> reload_out_reg: (reg:SI 88 SAR [140])
> reload_reg_rtx: (reg:SI 21 %r21)
>
>
> This happens because in df-scan.c at line 2676 (df_ref_record()) we have
>
> for (i = regno; i < endregno; i++)
> {
> ref = df_ref_create_structure (collection_rec, regno_reg_rtx[i], loc,
> bb, insn, ref_type, ref_flags);
>
> with regno = 22 and endregno = 23 with loc pointing to the
> (subreg:SI (reg:DI 21) 4) expression. df_ref_create_structure() will then set
> DF_REF_REG(ref) to regno_reg_rtx[22] and point DF_REF_LOC(ref) to the
> (subreg:SI (reg:DI 21) 4) expression. This goes wrong in df-problems.c at line 2832
> (df_note_bb_compute()), where we have
>
> rtx reg = (DF_REF_LOC (use))
> ? *DF_REF_REAL_LOC (use) : DF_REF_REG (use);
> old_dead_notes = df_set_note (REG_DEAD, insn, old_dead_notes, reg);
>
> #ifdef REG_DEAD_DEBUGGING
> df_print_note ("adding 4: ", insn, REG_NOTES (insn));
>
> and so dig into the subreg expression and find and use (reg:DI 21) instead
> of (reg:SI 22) for the REG_DEAD note.
>
> I can see a few ways to fix this:
>
> 1) Pass NULL for LOC to df_ref_create_structure().
> 2) Pass ®no_reg_rtx[i] for LOC to df_ref_create_structure().
> 3) Don't use DF_REF_REAL_LOC(use) if it looks like a bad idea.
>
> Comments? I tried option 3) and it seems to work:
>
> Index: /home/rask/cvssrc/owcc-gcc/gcc/df-problems.c
> ===================================================================
> --- /home/rask/cvssrc/owcc-gcc/gcc/df-problems.c (revision 129849)
> +++ /home/rask/cvssrc/owcc-gcc/gcc/df-problems.c (working copy)
> @@ -2829,7 +2829,10 @@
> && (!(DF_REF_FLAGS (use) & DF_REF_READ_WRITE))
> && (!df_ignore_stack_reg (uregno)))
> {
> - rtx reg = (DF_REF_LOC (use))
> + rtx reg = (DF_REF_LOC (use)
> + && (GET_MODE (*DF_REF_REAL_LOC (use))
> + == GET_MODE (DF_REF_REG (use)))
> + || !HARD_REGISTER_P (*DF_REF_REAL_LOC (use)))
> ? *DF_REF_REAL_LOC (use) : DF_REF_REG (use);
> old_dead_notes = df_set_note (REG_DEAD, insn, old_dead_notes, reg);
>
> (without the hard reg check I get RTL sharing errors.)
>
> live at bottom 3 [%r3] 20 [%r20] 21 [%r21] 25 [%r25] 30 [%r30] 31 [%r31]
> live before artificials out 3 [%r3] 20 [%r20] 21 [%r21] 25 [%r25] 30 [%r30] 31 [%r31]
> regular looking at def d-1 reg 21 bb 4 insn 74 flag 0x8 type 0x0 loc b7f7569c(b7f77288) chain { }
> regular looking at use u-1 reg 19 bb 4 insn 74 flag 0x8 type 0x1 loc b7f75684(b7f754f8) chain { }
> adding 4: 74 (expr_list:REG_DEAD (reg:SI 19 %r19 [139])
> (nil))
> regular looking at use u-1 reg 21 bb 4 insn 74 flag 0x8 type 0x1 loc b7f75688(b7f77288) chain { }
> regular looking at def d-1 reg 19 bb 4 insn 71 flag 0x8 type 0x0 loc b7f755ac(b7f754f8) chain { }
> regular looking at use u-1 reg 19 bb 4 insn 71 flag 0x8 type 0x1 loc b7f75594(b7f75408) chain { }
> regular looking at use u-1 reg 88 bb 4 insn 71 flag 0x8 type 0x1 loc b7f75580(b7f75528) chain { }
> adding 4: 71 (expr_list:REG_DEAD (reg:SI 88 SAR [140])
> (expr_list:REG_EQUAL (ashift:SI (reg:SI 19 %r19 [135])
> (subreg:SI (reg:DI 21 %r21 [137]) 4))
> (nil)))
> regular looking at def d-1 reg 88 bb 4 insn 70 flag 0x8 type 0x0 loc b7f75564(b7f75528) chain { }
> regular looking at use u-1 reg 22 bb 4 insn 70 flag 0x18 type 0x1 loc b7f75550(b7f75510) chain { }
> adding 4: 70 (expr_list:REG_DEAD (reg:SI 22 %r22)
> (nil))
> regular looking at def d-1 reg 21 bb 4 insn 73 flag 0x8 type 0x0 loc b7f7566c(b7f77288) chain { }
> regular looking at use u-1 reg 24 bb 4 insn 73 flag 0x18 type 0x1 loc b7f75658(b7f75600) chain { }
> adding 4: 73 (expr_list:REG_DEAD (reg:SI 24 %r24)
> (nil))
> regular looking at use u-1 reg 28 bb 4 insn 73 flag 0x8 type 0x1 loc b7f75654(b7f75618) chain { }
> adding 4: 73 (expr_list:REG_DEAD (reg:SI 28 %r28 [141])
> (expr_list:REG_DEAD (reg:SI 24 %r24)
> (nil)))
>
> (insn 73 72 70 4 longlong.c:19 (set (reg:SI 21 %r21 [orig:154+4 ] [154])
> (lshiftrt:SI (reg:SI 28 %r28 [141])
> (subreg:SI (reg:DI 23 %r23 [116]) 4))) 277 {lshrsi3} (expr_list:REG_DEAD (reg:SI 28 %r28 [141])
> (expr_list:REG_DEAD (reg:SI 24 %r24)
> (nil))))
>
> (insn 70 73 71 4 longlong.c:19 (set (reg:SI 88 SAR [140])
> (minus:SI (const_int 31 [0x1f])
> (subreg:SI (reg:DI 21 %r21 [137]) 4))) 184 {*pa.md:5698} (expr_list:REG_DEAD (reg:SI 22 %r22)
> (nil)))
>
> (insn 71 70 74 4 longlong.c:19 (set (reg:SI 19 %r19 [139])
> (ashift:SI (reg:SI 19 %r19 [135])
> (minus:SI (const_int 31 [0x1f])
> (reg:SI 88 SAR [140])))) 264 {zvdep32} (expr_list:REG_DEAD (reg:SI 88 SAR [140])
> (expr_list:REG_EQUAL (ashift:SI (reg:SI 19 %r19 [135])
> (subreg:SI (reg:DI 21 %r21 [137]) 4))
> (nil))))
>
> (insn 74 71 77 4 longlong.c:19 (set (reg:SI 21 %r21 [orig:154+4 ] [154])
> (ior:SI (reg:SI 19 %r19 [139])
> (reg:SI 21 %r21 [orig:154+4 ] [154]))) 210 {*pa.md:6217} (expr_list:REG_DEAD (reg:SI 19 %r19 [139])
> (nil)))
>
> Reloads for insn # 73
> Reload 0: reload_in (SI) = (reg:SI 24 %r24)
> SHIFT_REGS, RELOAD_FOR_INPUT (opnum = 2), can't combine
> reload_in_reg: (subreg:SI (reg:DI 23 %r23 [116]) 4)
> reload_reg_rtx: (reg:SI 88 SAR)
>
> Reloads for insn # 70
> Reload 0: reload_out (SI) = (reg:SI 88 SAR [140])
> GENERAL_REGS, RELOAD_FOR_OUTPUT (opnum = 0)
> reload_out_reg: (reg:SI 88 SAR [140])
> reload_reg_rtx: (reg:SI 22 %r22)
>
>
> (insn 73 133 70 4 longlong.c:19 (set (reg:SI 21 %r21 [orig:154+4 ] [154])
> (lshiftrt:SI (reg:SI 28 %r28 [141])
> (reg:SI 88 SAR))) 277 {lshrsi3} (nil))
>
> (insn 70 73 134 4 longlong.c:19 (set (reg:SI 22 %r22)
> (minus:SI (const_int 31 [0x1f])
> (reg:SI 22 %r22 [orig:137+4 ] [137]))) 184 {*pa.md:5698} (nil))
>
> (insn 134 70 71 4 longlong.c:19 (set (reg:SI 88 SAR [140])
> (reg:SI 22 %r22)) 70 {*pa.md:2542} (nil))
>
> (insn 71 134 74 4 longlong.c:19 (set (reg:SI 19 %r19 [139])
> (ashift:SI (reg:SI 19 %r19 [135])
> (minus:SI (const_int 31 [0x1f])
> (reg:SI 88 SAR [140])))) 264 {zvdep32} (expr_list:REG_EQUAL (ashift:SI (reg:SI 19 %r19 [135])
> (subreg:SI (reg:DI 21 %r21 [137]) 4))
> (nil)))
>
> (insn 74 71 77 4 longlong.c:19 (set (reg:SI 21 %r21 [orig:154+4 ] [154])
> (ior:SI (reg:SI 19 %r19 [139])
> (reg:SI 21 %r21 [orig:154+4 ] [154]))) 210 {*pa.md:6217} (nil))
>
> I don't understand much of PA-RISC asm, but compared with
> <URL:http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33732#c4> at least the
> number of instructions is now right:
>
> .L10:
> uaddcm %r0,%r23,%r21 ; 69 *pa.md:6342 [length = 8]
> uaddcm %r0,%r24,%r22
> mtsar %r24 ; 133 *pa.md:2542/8 [length = 4]
> ldw 16(%r20),%r19 ; 66 *pa.md:2542/6 [length = 4]
> ldw 4(%r25),%r28 ; 72 *pa.md:2542/6 [length = 4]
> zdep %r19,30,31,%r19 ; 67 *pa.md:6987 [length = 4]
> vshd %r0,%r28,%r21 ; 73 lshrsi3/1 [length = 4]
> subi 31,%r22,%r22 ; 70 *pa.md:5698/2 [length = 4]
> mtsar %r22 ; 134 *pa.md:2542/8 [length = 4]
> zvdep %r19,32,%r19 ; 71 zvdep32/1 [length = 4]
> b .L11 ; 140 jump [length = 4]
> or %r19,%r21,%r21 ; 74 *pa.md:6217 [length = 4]
> .L16:
>
>
i think that eric, in his earlier email, was on the right track. I did
not respond to his email because i view this as a reload issue and not a
dataflow issue.
This problem comes about because it is actually hard to keep dead notes
up to date without actually doing the backwards scan in the effected
basic block.
It is my hope that as vlad redoes the register allocator, it can be
transformed into something that works off backwards scans (as virtually
all other register allocators do) rather than forwards scans that depend
on these dead notes. In this way the ra could just be dead/unused note
free and then these problems go away.
Extending dataflow further into the ra/reload stack is actually quite
hard. It is something that i would never consider in stage 3. Even in
stage 1 it is difficult to do: local alloc builds tables that are used
later in the ra/reload stack. Redoing the dataflow within ra/reload
has in the past (before the dataflow commit) caused these tables to be
inconsistent. Turning dataflow off was the only way that we could avoid
messing up the existing stack.
I am hoping that some/all of this goes away with vlad's ra. If it does
not, we will reconsider the issues. For now, i think that eric has a
handle on the right direction.
kenny
More information about the Gcc-patches
mailing list