Re: PATCH: tune ARM's rtx_costs function

[James Lemke <jim at wasabisystems dot com>]

> >  	  
> > +	  if (arm_is_xscale)
> 
> This patch isn't against the trunk.  On the trunk arm_is_xscale has been 
> split into arm_tune_xscale and arm_arch_xscale.  You should be using the 
> former here.
I'll make a note.

> > +	    {
> > +	      unsigned HOST_WIDE_INT masked_const;
> > +
> > +	      /* The cost will be related to two insns.
> > +		 First a load of the constant (MOV or LDR), then a multiply. */
> > +	      cost = 2;
> > +	      if (! const_ok)
> > +		  cost += 1;	/* LDR is probably more expensive because
> 
> There should be and indent of two spaces here.
Dunno how that happened.  Will fix.

> > +		  		   of longer result latency. */
> > +	      masked_const = i & 0xFfff8000UL;
> 
> Don't capitalize the first character of a constant.  And although it's not 
> wrong on the trunk (now that we've switched to ISO C), the rest of the 
> code in arm.c does not use the UL suffix.  On the 3.3 branch the suffix 
> would still be 'verboten' because of the need for K+R compatibility.
Will fix.

> > +	      if (masked_const == 0UL || masked_const == 0xFfff8000UL)
> > +		  ;
> 
> Why the dead alternative here?  Since you only have one else clause it's 
> better to rewrite the condition so as to eliminate the empty statement.
I did this because both tests (above & below) match the Xscale docs
better, and the two tests are obviously similar.
However, not a big deal.  I will negate the first test to get rid of the
null clause.

> > +	      else
> > +		{
> > +	          masked_const = i & 0xF8000000UL;
> > +	          if (masked_const == 0UL || masked_const == 0xF8000000UL)
> > +		      cost += 1;
> > +	          else
> > +		      cost += 2;
> > +		}
> > +	      return cost;
> > +	    }
> > +
> >  	  /* Tune as appropriate.  */ 
> > -	  int booth_unit_size = ((tune_flags & FL_FAST_MULT) ? 8 : 2);
> > +	  cost = const_ok ? 4 : 8;
> > +	  booth_unit_size = ((tune_flags & FL_FAST_MULT) ? 8 : 2);
> >  	  
> >  	  for (j = 0; i && j < 32; j += booth_unit_size)
> >  	    {
> >  	      i >>= booth_unit_size;
> > -	      add_cost += 2;
> > +	      cost += 2;
> >  	    }
> >  
> > -	  return add_cost;
> > +	  return cost;
> >  	}
> >  
> >        return (((tune_flags & FL_FAST_MULT) ? 8 : 30)

-- 
James Lemke   jim@wasabisystems.com   Orillia, Ontario
http://www.wasabisystems.com