Re: PATCH: tune ARM's rtx_costs function

[Richard Earnshaw <rearnsha at arm dot com>]

> > This needs to be reformatted according to GNU coding standards.
> 
> Here are the re-formatted patches.  OK to commit?
> 
>  
> 2003-07-07  James Lemke <jim@wasabisystems.com>
>  
> 	* config/arm/arm.c (arm_rtx_costs): Improve for xscale multiply.
> 	* config/arm/arm.md (addsi3): Use new pseudo.
> 	* testsuite/gcc.dg/arm-g2.c: New file.

There are still problems here.  Mostly cosmetic.

>  	  
> +	  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.

> +	    {
> +	      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.

> +		  		   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.

> +	      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.

> +	      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)
> 
> Index: config/arm/arm.md
> ===================================================================
> RCS file: /cvsroot/gnu/gcc/config/arm/arm.md,v
> retrieving revision 1.3.2.4
> retrieving revision 1.3.2.5
> diff -u -r1.3.2.4 -r1.3.2.5
> --- config/arm/arm.md	1 Oct 2003 12:44:08 -0000	1.3.2.4
> +++ config/arm/arm.md	2 Oct 2003 17:38:10 -0000	1.3.2.5
> @@ -531,8 +531,7 @@
>    if (TARGET_ARM && GET_CODE (operands[2]) == CONST_INT)
>      {
>        arm_split_constant (PLUS, SImode, INTVAL (operands[2]), operands[0],
> -			  operands[1],
> -			  (no_new_pseudos ? 0 : preserve_subexpressions_p ()));
> +			  operands[1], ! no_new_pseudos);
>        DONE;
>      }
>    "
> 

See my previous mail.

R.