Re: [var-tracking] small speed-ups

[Jakub Jelinek <jakub@redhat.com>]

On Mon, Aug 22, 2011 at 01:30:33PM +0300, Dimitrios Apostolou wrote:
> --- gcc/var-tracking.c	2011-06-03 01:42:31 +0000
> +++ gcc/var-tracking.c	2011-08-22 09:52:08 +0000
> @@ -1069,14 +1067,14 @@ adjust_insn (basic_block bb, rtx insn)
>  static inline bool
>  dv_is_decl_p (decl_or_value dv)
>  {
> -  return !dv || (int) TREE_CODE ((tree) dv) != (int) VALUE;
> +  return !dv || ((int) TREE_CODE ((tree) dv) != (int) VALUE);
>  }

Why?

>  /* Return true if a decl_or_value is a VALUE rtl.  */
>  static inline bool
>  dv_is_value_p (decl_or_value dv)
>  {
> -  return dv && !dv_is_decl_p (dv);
> +  return !dv_is_decl_p (dv);
>  }

This is fine, though I don't think it makes any difference if var-tracking
is compiled with -O+ - gcc should be able to optimize the second
NULL/non-NULL check out.

> @@ -1191,7 +1189,7 @@ dv_uid2hash (dvuid uid)
>  static inline hashval_t
>  dv_htab_hash (decl_or_value dv)
>  {
> -  return dv_uid2hash (dv_uid (dv));
> +  return (hashval_t) (dv_uid (dv));
>  }

Why?  dv_uid2hash is an inline that does exactly that.

> @@ -1202,7 +1200,7 @@ variable_htab_hash (const void *x)
>  {
>    const_variable const v = (const_variable) x;
>  
> -  return dv_htab_hash (v->dv);
> +  return (hashval_t) (dv_uid (v->dv));
>  }

Why?

>  /* Compare the declaration of variable X with declaration Y.  */
> @@ -1211,9 +1209,8 @@ static int
>  variable_htab_eq (const void *x, const void *y)
>  {
>    const_variable const v = (const_variable) x;
> -  decl_or_value dv = CONST_CAST2 (decl_or_value, const void *, y);
>  
> -  return (dv_as_opaque (v->dv) == dv_as_opaque (dv));
> +  return (v->dv) == y;
>  }

Why?

> @@ -1397,19 +1398,40 @@ shared_var_p (variable var, shared_hash 
>  	  || shared_hash_shared (vars));
>  }
>  
> +/* Copy all variables from hash table SRC to hash table DST without rehashing
> +   any values.  */
> +
> +static htab_t
> +htab_dup (htab_t src)
> +{
> +  htab_t dst;
> +
> +  dst = (htab_t) xmalloc (sizeof (*src));
> +  memcpy (dst, src, sizeof (*src));
> +  dst->entries = (void **) xmalloc (src->size * sizeof (*src->entries));
> +  memcpy (dst->entries, src->entries,
> +	  src->size * sizeof (*src->entries));
> +  return dst;
> +}
> +

This certainly doesn't belong here, it should go into libiberty/hashtab.c
and prototype into include/hashtab.h.  It relies on hashtab.c
implementation details.

> @@ -2034,7 +2041,8 @@ val_resolve (dataflow_set *set, rtx val,
>  static void
>  dataflow_set_init (dataflow_set *set)
>  {
> -  init_attrs_list_set (set->regs);
> +  /* Initialize the set (array) SET of attrs to empty lists.  */
> +  memset (set->regs, 0, sizeof (set->regs));
>    set->vars = shared_hash_copy (empty_shared_hash);
>    set->stack_adjust = 0;
>    set->traversed_vars = NULL;

I'd say you should instead just implement init_attrs_list_set inline using
memset.

>    dst->vars = (shared_hash) pool_alloc (shared_hash_pool);
>    dst->vars->refcount = 1;
>    dst->vars->htab
> -    = htab_create (MAX (src1_elems, src2_elems), variable_htab_hash,
> +    = htab_create (2 * MAX (src1_elems, src2_elems), variable_htab_hash,
>  		   variable_htab_eq, variable_htab_free);

This looks wrong, 2 * max is definitely too much.

> @@ -8996,11 +9006,13 @@ vt_finalize (void)
>  
>    FOR_ALL_BB (bb)
>      {
> -      dataflow_set_destroy (&VTI (bb)->in);
> -      dataflow_set_destroy (&VTI (bb)->out);
> +      /* The "false" do_free parameter means to not bother to iterate and free
> +	 all hash table elements, since we'll destroy the pools. */
> +      dataflow_set_destroy (&VTI (bb)->in, false);
> +      dataflow_set_destroy (&VTI (bb)->out, false);
>        if (VTI (bb)->permp)
>  	{
> -	  dataflow_set_destroy (VTI (bb)->permp);
> +	  dataflow_set_destroy (VTI (bb)->permp, false);
>  	  XDELETE (VTI (bb)->permp);
>  	}
>      }
> 

How much does this actually speed things up (the not freeing pool allocated
stuff during finalizaqtion)?  Is it really worth it?

	Jakub