This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFC] Fix PR rtl-optimization/33732


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 &regno_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:

-- 
Rask Ingemann Lambertsen
Danish law requires addresses in e-mail to be logged and stored for a year


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]