Re: [RFC][patch] Make DCACHE_LINE runtime-settable

[Pedro Alves <pedro@codesourcery.com>]

Hi Paul,

On Monday 25 July 2011 19:32:51, Paul Pluzhnikov wrote:
>  
> -/* The size of a cache line.  Smaller values reduce the time taken to
> +/* The default size of a cache line.  Smaller values reduce the time taken to
>     read a single byte and make the cache more granular, but increase
>     overhead and reduce the effectiveness of the cache as a prefetcher.  */
> -#define LINE_SIZE_POWER 6
> -#define LINE_SIZE (1 << LINE_SIZE_POWER)
> +#define DCACHE_DEFAULT_LINE_SIZE 64
> +static unsigned dcache_line_size = DCACHE_DEFAULT_LINE_SIZE;
>  
>  /* Each cache block holds LINE_SIZE bytes of data
>     starting at a multiple-of-LINE_SIZE address.  */
>  
> -#define LINE_SIZE_MASK  ((LINE_SIZE - 1))
> -#define XFORM(x)       ((x) & LINE_SIZE_MASK)
> -#define MASK(x)         ((x) & ~LINE_SIZE_MASK)
> +#define LINE_SIZE_MASK(dcache)  ((dcache->line_size - 1))
> +#define XFORM(dcache, x)       ((x) & LINE_SIZE_MASK(dcache))
> +#define MASK(dcache, x)         ((x) & ~LINE_SIZE_MASK(dcache))

Missing space after LINE_SIZE_MASK and before `('.

>  
>  struct dcache_block
>  {
> @@ -93,8 +98,8 @@ struct dcache_block
>    struct dcache_block *next;
>  
>    CORE_ADDR addr;              /* address of data */
> -  gdb_byte data[LINE_SIZE];    /* bytes at given address */
>    int refs;                    /* # hits */
> +  gdb_byte data[0];            /* line_size bytes at given address */

Arrays of 0 length are not valid C90.  Please make that `gdb_byte data[1]'
and adjust allocation accordingly (using offsetof (..., data) instead of
sizeof, or just subtracting 1).

> +/* BLOCK_FUNC routine for dcache_free.  */
> +
> +static void
> +free_block (struct dcache_block *block, void *param)
> +{
> +  free (block);

xfree.

>  
> +static void
> +set_dcache_size (char *args, int from_tty,
> +                struct cmd_list_element *c)
> +{
> +  if (dcache_size <= 0)

Given:

> +static unsigned dcache_size = DCACHE_DEFAULT_SIZE;

That < 0 can't ever return true.

> +    {
> +      unsigned d = dcache_size;
> +      dcache_size = DCACHE_DEFAULT_SIZE;
> +      error (_("Invalid dcache size: %u (must be positive)."), d);

If you meant to support negatives in the setting, is printing the
number as unsigned your intention?  I think it'll look confusing?

> +  add_setshow_uinteger_cmd ("line-size", class_obscure,
> +                           &dcache_line_size, _("\
> +Set dcache line size in bytes (must be power of 2)."), _("\
> +Show dcache line size."),
> +                           NULL,
> +                           set_dcache_line_size,
> +                           NULL,
> +                           &dcache_set_list, &dcache_show_list);
> +  add_setshow_uinteger_cmd ("size", class_obscure,

... you've registered the command as ..._uinteger... so it all looks
like the "must be positive" bits are dead, and you just want to
forbit 0.

-- 
Pedro Alves