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]

Fix PR rtl-optimization/28243


It's a regression (reported by the Gentool folks) at -O2 -ftracer -fPIC 
present on 4.1 branch for x86.  The dataflow verification code in flow.c 
(verify_local_live_at_start) stops the compiler during life2 because the 
lifetime of pseudo 59 has been wrongly extended by the combiner.

Before combine:

(insn 17 80 79 1 (set (reg/v:DF 59 [ sa.53 ])
        (float_extend:DF (mem/u/c/i:SF (plus:SI (reg:SI 3 bx)
                    (const:SI (unspec:SI [
                                (symbol_ref/u:SI ("*.LC1") [flags 0x2])
                            ] 1))) [7 S4 A32]))) 86 {*extendsfdf2_i387} (nil)
    (expr_list:REG_EQUAL (const_double:DF 0 [0x0] 0.0 [0x0.0p+0])
        (nil)))

(insn 79 17 83 1 (set (reg:DF 74)
        (float_extend:DF (mem/u/c/i:SF (plus:SI (reg:SI 3 bx)
                    (const:SI (unspec:SI [
                                (symbol_ref/u:SI ("*.LC2") [flags 0x2])
                            ] 1))) [7 S4 A32]))) 86 {*extendsfdf2_i387} (nil)
    (expr_list:REG_EQUAL (const_double:DF -1275068416 [0xb4000000] 3.6e+2 
[0x0.b4p+9])
        (nil)))

(insn 83 79 84 1 (set (reg/v:DF 60 [ sa.52 ])
        (reg/v:DF 59 [ sa.53 ])) 64 {*movdf_integer} (insn_list:REG_DEP_TRUE 
17 (nil))
    (expr_list:REG_DEAD (reg/v:DF 59 [ sa.53 ])
        (expr_list:REG_EQUAL (const_double:DF 0 [0x0] 0.0 [0x0.0p+0])
            (nil))))

(insn 84 83 87 1 (set (reg/v:DF 59 [ sa.53 ])
        (reg/v:DF 58 [ sa.54 ])) 64 {*movdf_integer} (nil)
    (expr_list:REG_EQUAL (const_double:DF -1275068416 [0xb4000000] 9.0e+1 
[0x0.b4p+7])
        (nil)))

(insn 87 84 88 1 (parallel [
            (set (reg/f:SI 7 sp)
                (plus:SI (reg/f:SI 7 sp)
                    (const_int -8 [0xfffffff8])))
            (clobber (reg:CC 17 flags))
        ]) 148 {*addsi_1} (nil)
    (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))

(insn 88 87 89 1 (set (mem/i:DF (pre_dec:SI (reg/f:SI 7 sp)) [0 S8 A64])
        (reg/v:DF 58 [ sa.54 ])) 62 {*pushdf_integer} (insn_list:REG_DEP_TRUE 
87 (nil))
    (nil))

(insn 89 88 90 1 (set (mem/i:DF (pre_dec:SI (reg/f:SI 7 sp)) [0 S8 A64])
        (reg/v:DF 60 [ sa.52 ])) 62 {*pushdf_integer} (insn_list:REG_DEP_TRUE 
83 (nil))
    (expr_list:REG_DEAD (reg/v:DF 60 [ sa.52 ])
        (nil)))

[...]
;; End of basic block 1, registers live:
 3 [bx] 6 [bp] 7 [sp] 16 [argp] 20 [frame] 58 59 61 73 74


After combine:

(note 17 80 79 1 NOTE_INSN_DELETED)

(insn 79 17 83 1 (set (reg:DF 74)
        (float_extend:DF (mem/u/c/i:SF (plus:SI (reg:SI 3 bx)
                    (const:SI (unspec:SI [
                                (symbol_ref/u:SI ("*.LC2") [flags 0x2])
                            ] 1))) [7 S4 A32]))) 86 {*extendsfdf2_i387} (nil)
    (expr_list:REG_EQUAL (const_double:DF -1275068416 [0xb4000000] 3.6e+2 
[0x0.b4p+9])
        (nil)))

(note 83 79 84 1 NOTE_INSN_DELETED)

(note 84 83 87 1 NOTE_INSN_DELETED)

(insn 87 84 88 1 (parallel [
            (set (reg/f:SI 7 sp)
                (plus:SI (reg/f:SI 7 sp)
                    (const_int -8 [0xfffffff8])))
            (clobber (reg:CC 17 flags))
        ]) 148 {*addsi_1} (nil)
    (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))

(insn 88 87 89 1 (set (mem/i:DF (pre_dec:SI (reg/f:SI 7 sp)) [0 S8 A64])
        (reg/v:DF 58 [ sa.54 ])) 62 {*pushdf_integer} (insn_list:REG_DEP_TRUE 
87 (nil))
    (nil))

(insn 89 88 90 1 (set (mem/i:DF (pre_dec:SI (reg/f:SI 7 sp)) [0 S8 A64])
        (const_double:DF 0 [0x0] 0.0 [0x0.0p+0])) 62 {*pushdf_integer} (nil)
    (nil))


Insn 84 should not have been deleted because it is a reaching definition of 
pseudo 59 for the successor of basic block 1.  The problem comes from the 
famous distribute_notes, invoked on the REG_DEAD note of insn 83, which gets 
confused by the 2 contiguous live ranges of pseudo 59:

	      /* You might think you could search back from FROM_INSN
		 rather than from I3, but combine tries to split invalid
		 combined instructions.  This can result in the old I2
		 or I1 moving later in the insn sequence.  */
	      for (tem = PREV_INSN (i3); place == 0; tem = PREV_INSN (tem))
		{
		  if (! INSN_P (tem))
		    {
		      if (tem == BB_HEAD (bb))
			break;
		      continue;
		    }

		  /* If the register is being set at TEM, see if that is all
		     TEM is doing.  If so, delete TEM.  Otherwise, make this
		     into a REG_UNUSED note instead. Don't delete sets to
		     global register vars.  */
		  if ((REGNO (XEXP (note, 0)) >= FIRST_PSEUDO_REGISTER
		       || !global_regs[REGNO (XEXP (note, 0))])
		      && reg_set_p (XEXP (note, 0), PATTERN (tem)))

So the search starts at I3 (insn 89), FROM_INSN being insn 83, and moves 
backward until the beginning of the basic block.  The condition will be true 
for insn 84 and then for insn 17 so both will be deleted.

Now of course the idea of the code is to delete the definitions, reaching or 
not, of the use to which the REG_DEAD was attached, so certainly insn 17 but 
certainly not insn 84.

This code dates back to RMS (rev 357) so it's a bit surprising not to have run 
into this kind of problems before.  On the other hand, Alan recently fixed 
the equivalent problem for an intervening use instead of an intervening def:
  http://gcc.gnu.org/ml/gcc-patches/2005-11/msg00598.html
so perhaps recent modifications in the compiler have prompted it to surface.

The head comment in the above excerpt of code has been added by Alan and 
explains that it is not correct to always start the search at FROM_INSN. 
However my understanding is that this only applies to uses to which the 
REG_DEAD note could be again attached, not to sets because new defs for the 
REG_DEAD note cannot obviously be created from the original FROM_INSN.

Bootstrapped/regtested on x86_64-suse-linux, applied to the mainline and 4.1 
branch.


2006-09-12  Eric Botcazou  <ebotcazou@libertysurf.fr>

        PR rtl-optimization/28243
        * combine.c (distribute_notes) <REG_DEAD>: Do not consider SETs past
	the insn to which the note was originally attached.


2006-09-12  Eric Botcazou  <ebotcazou@libertysurf.fr>

        * gcc.dg/pr28243.c: New test.


-- 
Eric Botcazou
Index: combine.c
===================================================================
--- combine.c	(revision 116828)
+++ combine.c	(working copy)
@@ -12263,12 +12263,14 @@ distribute_notes (rtx notes, rtx from_in
 		      continue;
 		    }
 
-		  /* If the register is being set at TEM, see if that is all
-		     TEM is doing.  If so, delete TEM.  Otherwise, make this
-		     into a REG_UNUSED note instead. Don't delete sets to
-		     global register vars.  */
-		  if ((REGNO (XEXP (note, 0)) >= FIRST_PSEUDO_REGISTER
-		       || !global_regs[REGNO (XEXP (note, 0))])
+		  /* If TEM is a (reaching) definition of the use to which the
+		     note was attached, see if that is all TEM is doing.  If so,
+		     delete TEM.  Otherwise, make this into a REG_UNUSED note
+		     instead.  Don't delete sets to global register vars.  */
+		  if ((!from_insn
+		       || INSN_CUID (tem) < INSN_CUID (from_insn))
+		      && (REGNO (XEXP (note, 0)) >= FIRST_PSEUDO_REGISTER
+			  || !global_regs[REGNO (XEXP (note, 0))])
 		      && reg_set_p (XEXP (note, 0), PATTERN (tem)))
 		    {
 		      rtx set = single_set (tem);
/* PR rtl-optimization/28243 */
/* Reported by Mike Frysinger <vapier@gentoo.org> */

/* { dg-do compile } */
/* { dg-options "-O2 -ftracer -fPIC" } */

struct displayfuncs {
  void (*init) ();
} funcs;

struct gpsdisplay {
  struct displayfuncs *funcs;
};

static void PSMyArc(double cx, double cy, double radx, double rady, double sa,
		    double ta)
{
  double ea;
  double temp;
  ea = sa + ta;
  while (sa < ea) {
    temp = ((sa + 90) / 90) * 90;
    PSDoArc(cx, sa, ea < temp ? ea : temp);
    sa = temp;
  }
}

static void PSDrawElipse()
{
  float cx;
  float cy;
  float radx;
  float rady;
  if (radx != rady)
    PSMyArc(cx, cy, radx, rady, 0, 360);
}

static void PSDrawFillCircle()
{
  PSDrawElipse();
}

static struct displayfuncs psfuncs[] = {
  PSDrawFillCircle
};

void _GPSDraw_CreateDisplay()
{
  struct gpsdisplay *gdisp;
  gdisp->funcs = (void *)&psfuncs;
}

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