Patchwork sel-sched: forbid differing modes in substitution (PR 50205)

login
register
mail settings
Submitter Alexander Monakov
Date Sept. 7, 2011, 9:38 a.m.
Message ID <alpine.LNX.2.00.1109071325500.28498@monoid.intra.ispras.ru>
Download mbox | patch
Permalink /patch/113732/
State New
Headers show

Comments

Alexander Monakov - Sept. 7, 2011, 9:38 a.m.
Hello,

The patch repairs a problem when we attempt to substitute an insn like
(... (cmp (mem (reg:DI ax)) (reg:SI ax))) (note different modes) through
(set (reg:DI ax) (reg:DI dx)), which leaves the (reg:SI ax) part of the
comparison intact, causing an ICE later on when we notice that the dependency
on ax is still present.  As this is quite rare, we can simply forbid
substitution in such circumstances, much like substitution of multiple hard
reg references is forbidden now.

En passant, the patch simplifes the code a little, as we never try to
substitute anything but registers.

Bootstrapped and regtested on x86_64-linux with sel-sched enabled at -O2, OK?
(I'll add the testcase from Bugzilla when committing)

2011-09-07  Alexander Monakov  <amonakov@ispras.ru>

	PR rtl-optimization/50205
	* sel-sched.c (count_occurrences_1): Simplify on the assumption that
	p->x is a register.  Forbid substitution when the same register is
	found in a different mode.
	(count_occurrences_equiv): Assert that 'what' is a register.
Alexander Monakov - Oct. 10, 2011, 3:32 p.m.
Ping?

On Wed, 7 Sep 2011, Alexander Monakov wrote:

> Hello,
> 
> The patch repairs a problem when we attempt to substitute an insn like
> (... (cmp (mem (reg:DI ax)) (reg:SI ax))) (note different modes) through
> (set (reg:DI ax) (reg:DI dx)), which leaves the (reg:SI ax) part of the
> comparison intact, causing an ICE later on when we notice that the dependency
> on ax is still present.  As this is quite rare, we can simply forbid
> substitution in such circumstances, much like substitution of multiple hard
> reg references is forbidden now.
> 
> En passant, the patch simplifes the code a little, as we never try to
> substitute anything but registers.
> 
> Bootstrapped and regtested on x86_64-linux with sel-sched enabled at -O2, OK?
> (I'll add the testcase from Bugzilla when committing)
> 
> 2011-09-07  Alexander Monakov  <amonakov@ispras.ru>
> 
> 	PR rtl-optimization/50205
> 	* sel-sched.c (count_occurrences_1): Simplify on the assumption that
> 	p->x is a register.  Forbid substitution when the same register is
> 	found in a different mode.
> 	(count_occurrences_equiv): Assert that 'what' is a register.
> 
> diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
> index f11faca..2af01ae 100644
> --- a/gcc/sel-sched.c
> +++ b/gcc/sel-sched.c
> @@ -813,18 +813,12 @@ count_occurrences_1 (rtx *cur_rtx, void *arg)
>  {
>    rtx_search_arg_p p = (rtx_search_arg_p) arg;
>  
> -  /* The last param FOR_GCSE is true, because otherwise it performs excessive
> -    substitutions like
> -	r8 = r33
> -	r16 = r33
> -    for the last insn it presumes r33 equivalent to r8, so it changes it to
> -    r33.  Actually, there's no change, but it spoils debugging.  */
> -  if (exp_equiv_p (*cur_rtx, p->x, 0, true))
> -    {
> -      /* Bail out if we occupy more than one register.  */
> -      if (REG_P (*cur_rtx)
> -          && HARD_REGISTER_P (*cur_rtx)
> -          && hard_regno_nregs[REGNO(*cur_rtx)][GET_MODE (*cur_rtx)] > 1)
> +  if (REG_P (*cur_rtx) && REGNO (*cur_rtx) == REGNO (p->x))
> +    {
> +      /* Bail out if mode is different or more than one register is used.  */
> +      if (GET_MODE (*cur_rtx) != GET_MODE (p->x)
> +          || (HARD_REGISTER_P (*cur_rtx)
> +	      && hard_regno_nregs[REGNO(*cur_rtx)][GET_MODE (*cur_rtx)] > 1))
>          {
>            p->n = 0;
>            return 1;
> @@ -837,7 +831,6 @@ count_occurrences_1 (rtx *cur_rtx, void *arg)
>      }
>  
>    if (GET_CODE (*cur_rtx) == SUBREG
> -      && REG_P (p->x)
>        && (!REG_P (SUBREG_REG (*cur_rtx))
>  	  || REGNO (SUBREG_REG (*cur_rtx)) == REGNO (p->x)))
>      {
> @@ -859,6 +852,7 @@ count_occurrences_equiv (rtx what, rtx where)
>  {
>    struct rtx_search_arg arg;
>  
> +  gcc_assert (REG_P (what));
>    arg.x = what;
>    arg.n = 0;
>  
> 
>
Vladimir Makarov - Oct. 14, 2011, 3:45 p.m.
On 09/07/2011 05:38 AM, Alexander Monakov wrote:
> Hello,
>
> The patch repairs a problem when we attempt to substitute an insn like
> (... (cmp (mem (reg:DI ax)) (reg:SI ax))) (note different modes) through
> (set (reg:DI ax) (reg:DI dx)), which leaves the (reg:SI ax) part of the
> comparison intact, causing an ICE later on when we notice that the dependency
> on ax is still present.  As this is quite rare, we can simply forbid
> substitution in such circumstances, much like substitution of multiple hard
> reg references is forbidden now.
>
> En passant, the patch simplifes the code a little, as we never try to
> substitute anything but registers.
>
> Bootstrapped and regtested on x86_64-linux with sel-sched enabled at -O2, OK?
> (I'll add the testcase from Bugzilla when committing)
>
Ok.  Thanks for the patch.
> 2011-09-07  Alexander Monakov<amonakov@ispras.ru>
>
> 	PR rtl-optimization/50205
> 	* sel-sched.c (count_occurrences_1): Simplify on the assumption that
> 	p->x is a register.  Forbid substitution when the same register is
> 	found in a different mode.
> 	(count_occurrences_equiv): Assert that 'what' is a register.
>

Patch

diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index f11faca..2af01ae 100644
--- a/gcc/sel-sched.c
+++ b/gcc/sel-sched.c
@@ -813,18 +813,12 @@  count_occurrences_1 (rtx *cur_rtx, void *arg)
 {
   rtx_search_arg_p p = (rtx_search_arg_p) arg;
 
-  /* The last param FOR_GCSE is true, because otherwise it performs excessive
-    substitutions like
-	r8 = r33
-	r16 = r33
-    for the last insn it presumes r33 equivalent to r8, so it changes it to
-    r33.  Actually, there's no change, but it spoils debugging.  */
-  if (exp_equiv_p (*cur_rtx, p->x, 0, true))
-    {
-      /* Bail out if we occupy more than one register.  */
-      if (REG_P (*cur_rtx)
-          && HARD_REGISTER_P (*cur_rtx)
-          && hard_regno_nregs[REGNO(*cur_rtx)][GET_MODE (*cur_rtx)] > 1)
+  if (REG_P (*cur_rtx) && REGNO (*cur_rtx) == REGNO (p->x))
+    {
+      /* Bail out if mode is different or more than one register is used.  */
+      if (GET_MODE (*cur_rtx) != GET_MODE (p->x)
+          || (HARD_REGISTER_P (*cur_rtx)
+	      && hard_regno_nregs[REGNO(*cur_rtx)][GET_MODE (*cur_rtx)] > 1))
         {
           p->n = 0;
           return 1;
@@ -837,7 +831,6 @@  count_occurrences_1 (rtx *cur_rtx, void *arg)
     }
 
   if (GET_CODE (*cur_rtx) == SUBREG
-      && REG_P (p->x)
       && (!REG_P (SUBREG_REG (*cur_rtx))
 	  || REGNO (SUBREG_REG (*cur_rtx)) == REGNO (p->x)))
     {
@@ -859,6 +852,7 @@  count_occurrences_equiv (rtx what, rtx where)
 {
   struct rtx_search_arg arg;
 
+  gcc_assert (REG_P (what));
   arg.x = what;
   arg.n = 0;