Patchwork cosmetic change - simplify cse.c:preferable()

login
register
mail settings
Submitter Dimitrios Apostolou
Date July 8, 2012, 9:26 a.m.
Message ID <alpine.LNX.2.02.1207081038340.4288@localhost.localdomain>
Download mbox | patch
Permalink /patch/169626/
State New
Headers show

Comments

Dimitrios Apostolou - July 8, 2012, 9:26 a.m.
Hello,

I've had this patch some time now, it's simple and cosmetic only, I 
had done it while trying to understand expression costs in CSE. I 
think it's more readable than the previous one. FWIW it passed all tests 
on x86.


Thanks,
Dimitris
Dodji Seketeli - July 18, 2012, 7:36 p.m.
Hey Dimitrios,

I can't say much about your patch, so I am CC-ing the maintainers.

Thanks.

Dimitrios Apostolou <jimis@gmx.net> a écrit:

> Hello,
>
> I've had this patch some time now, it's simple and cosmetic only, I
> had done it while trying to understand expression costs in CSE. I
> think it's more readable than the previous one. FWIW it passed all
> tests on x86.
>
>
> Thanks,
> Dimitris
>
> === modified file 'gcc/cse.c'
> --- gcc/cse.c	2012-06-15 09:22:00 +0000
> +++ gcc/cse.c	2012-07-08 07:28:52 +0000
> @@ -713,32 +713,25 @@ approx_reg_cost (rtx x)
>  static int
>  preferable (int cost_a, int regcost_a, int cost_b, int regcost_b)
>  {
> -  /* First, get rid of cases involving expressions that are entirely
> -     unwanted.  */
> -  if (cost_a != cost_b)
> -    {
> -      if (cost_a == MAX_COST)
> -	return 1;
> -      if (cost_b == MAX_COST)
> -	return -1;
> -    }
> +  int cost_diff = cost_a - cost_b;
> +  int regcost_diff = regcost_a - regcost_b;
>  
> -  /* Avoid extending lifetimes of hardregs.  */
> -  if (regcost_a != regcost_b)
> +  if (cost_diff != 0)
>      {
> -      if (regcost_a == MAX_COST)
> -	return 1;
> -      if (regcost_b == MAX_COST)
> -	return -1;
> +      /* If none of the expressions are entirely unwanted */
> +      if ((cost_a != MAX_COST) && (cost_b != MAX_COST)
> +	  /* AND only one of the regs is HARD_REG */
> +	  && (regcost_diff != 0)
> +	  && ((regcost_a == MAX_COST) || (regcost_b == MAX_COST))
> +	  )
> +	/* Then avoid extending lifetime of HARD_REG */
> +	return regcost_diff;
> +
> +      return cost_diff;
>      }
>  
> -  /* Normal operation costs take precedence.  */
> -  if (cost_a != cost_b)
> -    return cost_a - cost_b;
> -  /* Only if these are identical consider effects on register pressure.  */
> -  if (regcost_a != regcost_b)
> -    return regcost_a - regcost_b;
> -  return 0;
> +  /* cost_a == costb, consider effects on register pressure */
> +  return regcost_diff;
>  }
>  
>  /* Internal function, to compute cost when X is not a register; called
>
Richard Guenther - July 19, 2012, 8:38 a.m.
On Wed, 18 Jul 2012, Dodji Seketeli wrote:

> Hey Dimitrios,
> 
> I can't say much about your patch, so I am CC-ing the maintainers.

I don't think it's any good or clearer to understand.

Richard.

> Thanks.
> 
> Dimitrios Apostolou <jimis@gmx.net> a ?crit:
> 
> > Hello,
> >
> > I've had this patch some time now, it's simple and cosmetic only, I
> > had done it while trying to understand expression costs in CSE. I
> > think it's more readable than the previous one. FWIW it passed all
> > tests on x86.
> >
> >
> > Thanks,
> > Dimitris
> >
> > === modified file 'gcc/cse.c'
> > --- gcc/cse.c	2012-06-15 09:22:00 +0000
> > +++ gcc/cse.c	2012-07-08 07:28:52 +0000
> > @@ -713,32 +713,25 @@ approx_reg_cost (rtx x)
> >  static int
> >  preferable (int cost_a, int regcost_a, int cost_b, int regcost_b)
> >  {
> > -  /* First, get rid of cases involving expressions that are entirely
> > -     unwanted.  */
> > -  if (cost_a != cost_b)
> > -    {
> > -      if (cost_a == MAX_COST)
> > -	return 1;
> > -      if (cost_b == MAX_COST)
> > -	return -1;
> > -    }
> > +  int cost_diff = cost_a - cost_b;
> > +  int regcost_diff = regcost_a - regcost_b;
> >  
> > -  /* Avoid extending lifetimes of hardregs.  */
> > -  if (regcost_a != regcost_b)
> > +  if (cost_diff != 0)
> >      {
> > -      if (regcost_a == MAX_COST)
> > -	return 1;
> > -      if (regcost_b == MAX_COST)
> > -	return -1;
> > +      /* If none of the expressions are entirely unwanted */
> > +      if ((cost_a != MAX_COST) && (cost_b != MAX_COST)
> > +	  /* AND only one of the regs is HARD_REG */
> > +	  && (regcost_diff != 0)
> > +	  && ((regcost_a == MAX_COST) || (regcost_b == MAX_COST))
> > +	  )
> > +	/* Then avoid extending lifetime of HARD_REG */
> > +	return regcost_diff;
> > +
> > +      return cost_diff;
> >      }
> >  
> > -  /* Normal operation costs take precedence.  */
> > -  if (cost_a != cost_b)
> > -    return cost_a - cost_b;
> > -  /* Only if these are identical consider effects on register pressure.  */
> > -  if (regcost_a != regcost_b)
> > -    return regcost_a - regcost_b;
> > -  return 0;
> > +  /* cost_a == costb, consider effects on register pressure */
> > +  return regcost_diff;
> >  }
> >  
> >  /* Internal function, to compute cost when X is not a register; called
> >
> 
>
Dimitrios Apostolou - Aug. 3, 2012, 9:14 p.m.
On Thu, 19 Jul 2012, Richard Guenther wrote:
>
> I don't think it's any good or clearer to understand.

Hi Richi, I had forgotten I prepared this for PR #19832, maybe you want to 
take a look. FWIW, with my patch applied there is a difference of ~3 M 
instr, which is almost unmeasurable in time. But we can close the PR even 
with the simple patch Cristophe posted, since mine does not make 
things clearer.


Thanks,
Dimitris
Richard Guenther - Aug. 6, 2012, 11:35 a.m.
On Sat, 4 Aug 2012, Dimitrios Apostolou wrote:

> On Thu, 19 Jul 2012, Richard Guenther wrote:
> > 
> > I don't think it's any good or clearer to understand.
> 
> Hi Richi, I had forgotten I prepared this for PR #19832, maybe you want to
> take a look. FWIW, with my patch applied there is a difference of ~3 M instr,
> which is almost unmeasurable in time. But we can close the PR even with the
> simple patch Cristophe posted, since mine does not make things clearer.

Christophes patch is ok.  We still want to if-convert this in the
compiler though.

Thanks,
Richard.

Patch

=== modified file 'gcc/cse.c'
--- gcc/cse.c	2012-06-15 09:22:00 +0000

+++ gcc/cse.c	2012-07-08 07:28:52 +0000

@@ -713,32 +713,25 @@  approx_reg_cost (rtx x)

 static int
 preferable (int cost_a, int regcost_a, int cost_b, int regcost_b)
 {
-  /* First, get rid of cases involving expressions that are entirely

-     unwanted.  */

-  if (cost_a != cost_b)

-    {

-      if (cost_a == MAX_COST)

-	return 1;

-      if (cost_b == MAX_COST)

-	return -1;

-    }

+  int cost_diff = cost_a - cost_b;

+  int regcost_diff = regcost_a - regcost_b;

 
-  /* Avoid extending lifetimes of hardregs.  */

-  if (regcost_a != regcost_b)

+  if (cost_diff != 0)

     {
-      if (regcost_a == MAX_COST)

-	return 1;

-      if (regcost_b == MAX_COST)

-	return -1;

+      /* If none of the expressions are entirely unwanted */

+      if ((cost_a != MAX_COST) && (cost_b != MAX_COST)

+	  /* AND only one of the regs is HARD_REG */

+	  && (regcost_diff != 0)

+	  && ((regcost_a == MAX_COST) || (regcost_b == MAX_COST))

+	  )

+	/* Then avoid extending lifetime of HARD_REG */

+	return regcost_diff;

+

+      return cost_diff;

     }
 
-  /* Normal operation costs take precedence.  */

-  if (cost_a != cost_b)

-    return cost_a - cost_b;

-  /* Only if these are identical consider effects on register pressure.  */

-  if (regcost_a != regcost_b)

-    return regcost_a - regcost_b;

-  return 0;

+  /* cost_a == costb, consider effects on register pressure */

+  return regcost_diff;

 }
 
 /* Internal function, to compute cost when X is not a register; called