cosmetic change - simplify cse.c:preferable()

Submitted by Dimitrios Apostolou on July 8, 2012, 9:26 a.m.

Details

Message ID alpine.LNX.2.02.1207081038340.4288@localhost.localdomain
State New
Headers show

Commit Message

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

Comments

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 hide | download patch | download mbox

=== 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