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/28726


It's a regression present at -O2 -fsched2-use-superblocks on 4.1 branch (and 
latent on mainline) for STACK_REGS platforms (x86), a direct fallout of

2005-04-25  Roger Sayle  <roger@eyesopen.com>

	* sched-deps.c (sched_analyze_1): On STACK_REGS targets, x87, treat
	all writes to any stack register as a read/write dependency on
	FIRST_STACK_REG.
	(sched_analyze_2): Likewise, for reads from any stack register.


The 2nd scheduling pass hoists the insn

(insn:HI 31 84 32 2 (set (reg/v:DF 9 st(1) [orig:61 retval ] [61])
        (plus:DF (reg/v:DF 9 st(1) [orig:61 retval ] [61])
            (reg:DF 8 st [64]))) 407 {*fop_df_comm_i387} 
(insn_list:REG_DEP_TRUE 30 (nil))
    (expr_list:REG_DEAD (reg:DF 8 st [64])
        (nil)))

out of a loop, which is a bit annoying since st(1) is a GIV of the loop. :-)


Superblocks scheduling (aka extended basic blocks scheduling) uses dataflow 
information computed by earlier passes to move instructions around branches, 
i.e. the dependency information for branches is computed from the registers 
live on entry in target blocks and from the registers set or clobbered by 
instructions prior to the branches.

The above patch was aimed to prevent x87 instructions from being reordered by 
introducing an automatic dependency on st(0) for all of them:
  http://gcc.gnu.org/ml/gcc-patches/2005-04/msg01866.html
The problem is that it didn't introduce new defs/uses for the instructions, it 
simply replaced all st(n) defs/uses by a unique def/use of st(0)!

Bootstrapped/regtested on i586-redhat-linux, applied to the mainline and 4.1 
branch.  I verified that the mainline still generates the optimal code on 
Roger's testcase.


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

        PR rtl-optimization/28726
	* sched-deps.c (sched_analyze_reg): New function extracted from...
	(sched_analyze_1): ...here.  Call it to analyze references to
	registers.  Treat again writes to a stack register as writing to the
	register.
	(sched_analyze_2): ...and here.  Call it to analyze references to
	registers.  Treat again reads of a stack register as reading the
	register.


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

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


-- 
Eric Botcazou
Index: sched-deps.c
===================================================================
--- sched-deps.c	(revision 116809)
+++ sched-deps.c	(working copy)
@@ -715,6 +715,78 @@ flush_pending_lists (struct deps *deps, 
   deps->pending_flush_length = 1;
 }
 
+/* Analyze a single reference to register (reg:MODE REGNO) in INSN.
+   The type of the reference is specified by REF and can be SET,
+   CLOBBER, PRE_DEC, POST_DEC, PRE_INC, POST_INC or USE.  */
+
+static void
+sched_analyze_reg (struct deps *deps, int regno, enum machine_mode mode,
+		   enum rtx_code ref, rtx insn)
+{
+  /* A hard reg in a wide mode may really be multiple registers.
+     If so, mark all of them just like the first.  */
+  if (regno < FIRST_PSEUDO_REGISTER)
+    {
+      int i = hard_regno_nregs[regno][mode];
+      if (ref == SET)
+	{
+	  while (--i >= 0)
+	    SET_REGNO_REG_SET (reg_pending_sets, regno + i);
+	}
+      else if (ref == USE)
+	{
+	  while (--i >= 0)
+	    SET_REGNO_REG_SET (reg_pending_uses, regno + i);
+	}
+      else
+	{
+	  while (--i >= 0)
+	    SET_REGNO_REG_SET (reg_pending_clobbers, regno + i);
+	}
+    }
+
+  /* ??? Reload sometimes emits USEs and CLOBBERs of pseudos that
+     it does not reload.  Ignore these as they have served their
+     purpose already.  */
+  else if (regno >= deps->max_reg)
+    {
+      enum rtx_code code = GET_CODE (PATTERN (insn));
+      gcc_assert (code == USE || code == CLOBBER);
+    }
+
+  else
+    {
+      if (ref == SET)
+	SET_REGNO_REG_SET (reg_pending_sets, regno);
+      else if (ref == USE)
+	SET_REGNO_REG_SET (reg_pending_uses, regno);
+      else
+	SET_REGNO_REG_SET (reg_pending_clobbers, regno);
+
+      /* Pseudos that are REG_EQUIV to something may be replaced
+	 by that during reloading.  We need only add dependencies for
+	the address in the REG_EQUIV note.  */
+      if (!reload_completed && get_reg_known_equiv_p (regno))
+	{
+	  rtx t = get_reg_known_value (regno);
+	  if (MEM_P (t))
+	    sched_analyze_2 (deps, XEXP (t, 0), insn);
+	}
+
+      /* Don't let it cross a call after scheduling if it doesn't
+	 already cross one.  */
+      if (REG_N_CALLS_CROSSED (regno) == 0)
+	{
+	  if (ref == USE)
+	    deps->sched_before_next_call
+	      = alloc_INSN_LIST (insn, deps->sched_before_next_call);
+	  else
+	    add_dependence_list (insn, deps->last_function_call, 1,
+				 REG_DEP_ANTI);
+	}
+    }
+}
+
 /* Analyze a single SET, CLOBBER, PRE_DEC, POST_DEC, PRE_INC or POST_INC
    rtx, X, creating all dependencies generated by the write to the
    destination of X, and reads of everything mentioned.  */
@@ -722,7 +794,6 @@ flush_pending_lists (struct deps *deps, 
 static void
 sched_analyze_1 (struct deps *deps, rtx x, rtx insn)
 {
-  int regno;
   rtx dest = XEXP (x, 0);
   enum rtx_code code = GET_CODE (x);
 
@@ -771,64 +842,21 @@ sched_analyze_1 (struct deps *deps, rtx 
 
   if (REG_P (dest))
     {
-      regno = REGNO (dest);
+      int regno = REGNO (dest);
+      enum machine_mode mode = GET_MODE (dest);
+
+      sched_analyze_reg (deps, regno, mode, code, insn);
 
 #ifdef STACK_REGS
       /* Treat all writes to a stack register as modifying the TOS.  */
       if (regno >= FIRST_STACK_REG && regno <= LAST_STACK_REG)
 	{
-	  SET_REGNO_REG_SET (reg_pending_uses, FIRST_STACK_REG);
-	  regno = FIRST_STACK_REG;
+	  /* Avoid analyzing the same register twice.  */
+	  if (regno != FIRST_STACK_REG)
+	    sched_analyze_reg (deps, FIRST_STACK_REG, mode, code, insn);
+	  sched_analyze_reg (deps, FIRST_STACK_REG, mode, USE, insn);
 	}
 #endif
-
-      /* A hard reg in a wide mode may really be multiple registers.
-         If so, mark all of them just like the first.  */
-      if (regno < FIRST_PSEUDO_REGISTER)
-	{
-	  int i = hard_regno_nregs[regno][GET_MODE (dest)];
-	  if (code == SET)
-	    {
-	      while (--i >= 0)
-		SET_REGNO_REG_SET (reg_pending_sets, regno + i);
-	    }
-	  else
-	    {
-	      while (--i >= 0)
-		SET_REGNO_REG_SET (reg_pending_clobbers, regno + i);
-	    }
-	}
-      /* ??? Reload sometimes emits USEs and CLOBBERs of pseudos that
-	 it does not reload.  Ignore these as they have served their
-	 purpose already.  */
-      else if (regno >= deps->max_reg)
-	{
-	  gcc_assert (GET_CODE (PATTERN (insn)) == USE
-		      || GET_CODE (PATTERN (insn)) == CLOBBER);
-	}
-      else
-	{
-	  if (code == SET)
-	    SET_REGNO_REG_SET (reg_pending_sets, regno);
-	  else
-	    SET_REGNO_REG_SET (reg_pending_clobbers, regno);
-
-	  /* Pseudos that are REG_EQUIV to something may be replaced
-	     by that during reloading.  We need only add dependencies for
-	     the address in the REG_EQUIV note.  */
-	  if (!reload_completed && get_reg_known_equiv_p (regno))
-	    {
-	      rtx t = get_reg_known_value (regno);
-	      if (MEM_P (t))
-	        sched_analyze_2 (deps, XEXP (t, 0), insn);
-	    }
-
-	  /* Don't let it cross a call after scheduling if it doesn't
-	     already cross one.  */
-	  if (REG_N_CALLS_CROSSED (regno) == 0)
-	    add_dependence_list (insn, deps->last_function_call, 1,
-				 REG_DEP_ANTI);
-	}
     }
   else if (MEM_P (dest))
     {
@@ -935,51 +963,20 @@ sched_analyze_2 (struct deps *deps, rtx 
     case REG:
       {
 	int regno = REGNO (x);
+	enum machine_mode mode = GET_MODE (x);
+
+	sched_analyze_reg (deps, regno, mode, USE, insn);
 
 #ifdef STACK_REGS
       /* Treat all reads of a stack register as modifying the TOS.  */
       if (regno >= FIRST_STACK_REG && regno <= LAST_STACK_REG)
 	{
-	  SET_REGNO_REG_SET (reg_pending_sets, FIRST_STACK_REG);
-	  regno = FIRST_STACK_REG;
+	  /* Avoid analyzing the same register twice.  */
+	  if (regno != FIRST_STACK_REG)
+	    sched_analyze_reg (deps, FIRST_STACK_REG, mode, USE, insn);
+	  sched_analyze_reg (deps, FIRST_STACK_REG, mode, SET, insn);
 	}
 #endif
-
-	if (regno < FIRST_PSEUDO_REGISTER)
-	  {
-	    int i = hard_regno_nregs[regno][GET_MODE (x)];
-	    while (--i >= 0)
-	      SET_REGNO_REG_SET (reg_pending_uses, regno + i);
-	  }
-	/* ??? Reload sometimes emits USEs and CLOBBERs of pseudos that
-	   it does not reload.  Ignore these as they have served their
-	   purpose already.  */
-	else if (regno >= deps->max_reg)
-	  {
-	    gcc_assert (GET_CODE (PATTERN (insn)) == USE
-			|| GET_CODE (PATTERN (insn)) == CLOBBER);
-	  }
-	else
-	  {
-	    SET_REGNO_REG_SET (reg_pending_uses, regno);
-
-	    /* Pseudos that are REG_EQUIV to something may be replaced
-	       by that during reloading.  We need only add dependencies for
-	       the address in the REG_EQUIV note.  */
-	    if (!reload_completed && get_reg_known_equiv_p (regno))
-	      {
-		rtx t = get_reg_known_value (regno);
-		if (MEM_P (t))
-		  sched_analyze_2 (deps, XEXP (t, 0), insn);
-	      }
-
-	    /* If the register does not already cross any calls, then add this
-	       insn to the sched_before_next_call list so that it will still
-	       not cross calls after scheduling.  */
-	    if (REG_N_CALLS_CROSSED (regno) == 0)
-	      deps->sched_before_next_call
-		= alloc_INSN_LIST (insn, deps->sched_before_next_call);
-	  }
 	return;
       }
 
/* PR rtl-optimization/28726 */
/* Origin: Sigurd Schneider <sg313d@gmail.com> */

/* { dg-do run } */
/* { dg-options "-O2 -fsched2-use-superblocks" } */

extern void abort (void);

static double my_loop(void) __attribute__((noinline));

static double my_loop(void)
{
  double retval = 0.0;
  const unsigned char *start = "\005\b\000";
  const unsigned char *const end = start + 2;

  while (start < end)
    retval += *start++;

  return retval;
}

int main(void)
{
  if (my_loop() != 13.0)
    abort ();

  return 0;
}

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