public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [var-tracking] small speed-ups
@ 2011-08-22 11:38 Dimitrios Apostolou
  2011-08-22 12:21 ` [var-tracking] [not-good!] disable shared_hash and other simplifications Dimitrios Apostolou
  2011-08-22 13:44 ` [var-tracking] small speed-ups Jakub Jelinek
  0 siblings, 2 replies; 9+ messages in thread
From: Dimitrios Apostolou @ 2011-08-22 11:38 UTC (permalink / raw)
  To: gcc-patches; +Cc: jimis, Steven Bosscher, Alexandre Oliva

[-- Attachment #1: Type: TEXT/PLAIN, Size: 2179 bytes --]

Hello list,

this patch has all of my changes to var-tracking that I believe are worth 
keeping. These are all minor changes not touching algorithmic issues, I 
lost much time trying to understand what is actually done in 
var-tracking.c but I still don't get it. I wish there was some document 
describing current implementation. I'd appreciate all help I can get here.

Bootstrapped/tested on x86_64.

Thanks to lxo for helping me, hopefully his plan for better var-tracking 
throughout all optimisation passes will be implemented. This is the only 
var-tracking related doc I could find... ( 
http://gcc.gnu.org/wiki/Var_Tracking_Assignments).

This patch also includes minor stylistic changes that made the code just a 
tiny bit more accessible to me, the indirection was so much that it hardly 
reminded me of C, let me know if I should remove these parts. :-s For the 
sake of completion I'll also post a follow-up patch where I 
delete/simplify a big part of var-tracking, unfortunately with some impact 
on performance.


2011-08-22  Dimitrios Apostolou  <jimis@gmx.net>

 	* var-tracking.c (init_attrs_list_set): Remove function, instead
 	use a memset() call to zero the attr list in...
 	(dataflow_set_init).
 	(vars_copy): Remove function because inserting each element into a
 	new hash table was too costly. Replaced with the ...
 	(htab_dup): ... new function that only does a memcpy() of the
 	element table in htab_t, without rehashing any elements.
 	(shared_hash_unshare): Replace the vars_copy() call with
 	htab_dup(), plus do a little extra work (reference counting) which
 	was in vars_copy.
 	(shared_hash_destroy, dataflow_set_destroy): Add an extra
 	"do_free" bool argument, to avoid iterating over hash tables
 	freeing elements, when not needed.
 	(vt_finalize, vt_emit_notes): Call the above with do_free=false
 	since all pools will be freed later.
 	(dataflow_set_clear, dataflow_set_copy, dataflow_set_union)
 	(dataflow_set_merge, dataflow_set_different, compute_bb_dataflow)
 	(vt_find_locations): Call shared_hash_destroy with do_free=true.
 	(attrs_list_copy): Do not free destination list but reuse already
 	allocated elements if possible.

[-- Attachment #2: Type: TEXT/plain, Size: 10901 bytes --]

=== modified file 'gcc/var-tracking.c'
--- gcc/var-tracking.c	2011-06-03 01:42:31 +0000
+++ gcc/var-tracking.c	2011-08-22 09:52:08 +0000
@@ -414,7 +414,6 @@ static hashval_t variable_htab_hash (con
 static int variable_htab_eq (const void *, const void *);
 static void variable_htab_free (void *);
 
-static void init_attrs_list_set (attrs *);
 static void attrs_list_clear (attrs *);
 static attrs attrs_list_member (attrs, decl_or_value, HOST_WIDE_INT);
 static void attrs_list_insert (attrs *, decl_or_value, HOST_WIDE_INT, rtx);
@@ -423,7 +422,6 @@ static void attrs_list_union (attrs *, a
 
 static void **unshare_variable (dataflow_set *set, void **slot, variable var,
 				enum var_init_status);
-static void vars_copy (htab_t, htab_t);
 static tree var_debug_decl (tree);
 static void var_reg_set (dataflow_set *, rtx, enum var_init_status, rtx);
 static void var_reg_delete_and_set (dataflow_set *, rtx, bool,
@@ -447,7 +445,7 @@ static bool variable_part_different_p (v
 static bool onepart_variable_different_p (variable, variable);
 static bool variable_different_p (variable, variable);
 static bool dataflow_set_different (dataflow_set *, dataflow_set *);
-static void dataflow_set_destroy (dataflow_set *);
+static void dataflow_set_destroy (dataflow_set *, bool);
 
 static bool contains_symbol_ref (rtx);
 static bool track_expr_p (tree, bool);
@@ -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);
 }
 
 /* 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);
 }
 
 /* Return the decl in the decl_or_value.  */
@@ -1092,7 +1090,7 @@ static inline rtx
 dv_as_value (decl_or_value dv)
 {
   gcc_checking_assert (dv_is_value_p (dv));
-  return (rtx)dv;
+  return (rtx) dv;
 }
 
 /* Return the opaque pointer in the decl_or_value.  */
@@ -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));
 }
 
 /* The hash function for variable_htab, computes the hash value
@@ -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));
 }
 
 /* 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;
 }
 
 /* Free the element of VARIABLE_HTAB (its type is struct variable_def).  */
@@ -1265,17 +1262,6 @@ value_chain_htab_eq (const void *x, cons
   return dv_as_opaque (v->dv) == dv_as_opaque (dv);
 }
 
-/* Initialize the set (array) SET of attrs to empty lists.  */
-
-static void
-init_attrs_list_set (attrs *set)
-{
-  int i;
-
-  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
-    set[i] = NULL;
-}
-
 /* Make the list *LISTP empty.  */
 
 static void
@@ -1323,18 +1309,33 @@ attrs_list_insert (attrs *listp, decl_or
 static void
 attrs_list_copy (attrs *dstp, attrs src)
 {
-  attrs n;
+  attrs n, next, *nextp;
 
-  attrs_list_clear (dstp);
-  for (; src; src = src->next)
+  /* Copy to already existing nodes of *dstp, allocating new only if needed */
+  nextp = dstp;
+  while (src)
     {
-      n = (attrs) pool_alloc (attrs_pool);
+      if (*nextp == NULL)
+	{
+	  (*nextp) = (attrs) pool_alloc (attrs_pool);
+	  (*nextp)->next = NULL;
+	}
+      n = *nextp;
       n->loc = src->loc;
       n->dv = src->dv;
       n->offset = src->offset;
-      n->next = *dstp;
-      *dstp = n;
+      nextp = &n->next;
+      src = src->next;
     }
+  /* Free remaining elements of *dstp, if it was longer than src. */
+  n = *nextp;
+  while (n != NULL)
+    {
+      next = n->next;
+      pool_free (attrs_pool, n);
+      n = next;
+    }
+  (*nextp) = NULL;
 }
 
 /* Add all nodes from SRC which are not in *DSTP to *DSTP.  */
@@ -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;
+}
+
 /* Copy variables into a new hash table.  */
 
 static shared_hash
 shared_hash_unshare (shared_hash vars)
 {
+  variable var;
+  htab_iterator hi;
   shared_hash new_vars = (shared_hash) pool_alloc (shared_hash_pool);
+
   gcc_assert (vars->refcount > 1);
   new_vars->refcount = 1;
-  new_vars->htab
-    = htab_create (htab_elements (vars->htab) + 3, variable_htab_hash,
-		   variable_htab_eq, variable_htab_free);
-  vars_copy (new_vars->htab, vars->htab);
+  new_vars->htab = htab_dup (vars->htab);
+  FOR_EACH_HTAB_ELEMENT (new_vars->htab, var, variable, hi)
+    {
+      var->refcount++;
+    }
   vars->refcount--;
+
   return new_vars;
 }
 
@@ -1426,13 +1448,17 @@ shared_hash_copy (shared_hash vars)
    anymore.  */
 
 static void
-shared_hash_destroy (shared_hash vars)
+shared_hash_destroy (shared_hash vars, bool do_free)
 {
   gcc_checking_assert (vars->refcount > 0);
   if (--vars->refcount == 0)
     {
+      if (!do_free)
+	/* Dirty hack to prevent htab_delete() iterating over all elements */
+	vars->htab->del_f = NULL;
       htab_delete (vars->htab);
-      pool_free (shared_hash_pool, vars);
+      if (do_free)
+	pool_free (shared_hash_pool, vars);
     }
 }
 
@@ -1593,25 +1619,6 @@ unshare_variable (dataflow_set *set, voi
   return slot;
 }
 
-/* Copy all variables from hash table SRC to hash table DST.  */
-
-static void
-vars_copy (htab_t dst, htab_t src)
-{
-  htab_iterator hi;
-  variable var;
-
-  FOR_EACH_HTAB_ELEMENT (src, var, variable, hi)
-    {
-      void **dstp;
-      var->refcount++;
-      dstp = htab_find_slot_with_hash (dst, var->dv,
-				       dv_htab_hash (var->dv),
-				       INSERT);
-      *dstp = var;
-    }
-}
-
 /* Map a decl to its main debug decl.  */
 
 static inline tree
@@ -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;
@@ -2050,7 +2058,7 @@ dataflow_set_clear (dataflow_set *set)
   for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
     attrs_list_clear (&set->regs[i]);
 
-  shared_hash_destroy (set->vars);
+  shared_hash_destroy (set->vars, true);
   set->vars = shared_hash_copy (empty_shared_hash);
 }
 
@@ -2064,7 +2072,7 @@ dataflow_set_copy (dataflow_set *dst, da
   for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
     attrs_list_copy (&dst->regs[i], src->regs[i]);
 
-  shared_hash_destroy (dst->vars);
+  shared_hash_destroy (dst->vars, true);
   dst->vars = shared_hash_copy (src->vars);
   dst->stack_adjust = src->stack_adjust;
 }
@@ -2489,7 +2497,7 @@ dataflow_set_union (dataflow_set *dst, d
 
   if (dst->vars == empty_shared_hash)
     {
-      shared_hash_destroy (dst->vars);
+      shared_hash_destroy (dst->vars, true);
       dst->vars = shared_hash_copy (src->vars);
     }
   else
@@ -3711,11 +3719,11 @@ dataflow_set_merge (dataflow_set *dst, d
   src2_elems = htab_elements (shared_hash_htab (src2->vars));
   dataflow_set_init (dst);
   dst->stack_adjust = cur.stack_adjust;
-  shared_hash_destroy (dst->vars);
+  shared_hash_destroy (dst->vars, true);
   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);
 
   for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
@@ -3734,7 +3742,7 @@ dataflow_set_merge (dataflow_set *dst, d
   if (dsm.src_onepart_cnt)
     dst_can_be_shared = false;
 
-  dataflow_set_destroy (src1);
+  dataflow_set_destroy (src1, true);
 }
 
 /* Mark register equivalences.  */
@@ -4503,14 +4511,15 @@ dataflow_set_different (dataflow_set *ol
 /* Free the contents of dataflow set SET.  */
 
 static void
-dataflow_set_destroy (dataflow_set *set)
+dataflow_set_destroy (dataflow_set *set, bool do_free)
 {
   int i;
 
-  for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
-    attrs_list_clear (&set->regs[i]);
+  if (do_free)
+    for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
+      attrs_list_clear (&set->regs[i]);
 
-  shared_hash_destroy (set->vars);
+  shared_hash_destroy (set->vars, do_free);
   set->vars = NULL;
 }
 
@@ -6339,7 +6348,7 @@ compute_bb_dataflow (basic_block bb)
 #endif
     }
   changed = dataflow_set_different (&old_out, out);
-  dataflow_set_destroy (&old_out);
+  dataflow_set_destroy (&old_out, true);
   return changed;
 }
 
@@ -6456,7 +6465,7 @@ vt_find_locations (void)
 #endif
 		      if (dst_can_be_shared)
 			{
-			  shared_hash_destroy (in->vars);
+			  shared_hash_destroy (in->vars, true);
 			  in->vars = shared_hash_copy (first_out->vars);
 			}
 		    }
@@ -8343,7 +8352,8 @@ vt_emit_notes (void)
       gcc_assert (htab_elements (value_chains) == 0);
     }
 #endif
-  dataflow_set_destroy (&cur);
+  /* don't free memory here, we'll free pools right after in vt_finalize */
+  dataflow_set_destroy (&cur, false);
 
   if (MAY_HAVE_DEBUG_INSNS)
     {
@@ -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);
 	}
     }


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-08-23 14:37 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-22 11:38 [var-tracking] small speed-ups Dimitrios Apostolou
2011-08-22 12:21 ` [var-tracking] [not-good!] disable shared_hash and other simplifications Dimitrios Apostolou
2011-08-22 12:57   ` Jakub Jelinek
2011-08-22 18:53     ` Steven Bosscher
2011-08-22 18:55       ` Jakub Jelinek
2011-08-22 13:44 ` [var-tracking] small speed-ups Jakub Jelinek
2011-08-23 12:36   ` Dimitrios Apostolou
2011-08-23 14:51     ` Jakub Jelinek
2011-08-23 14:56       ` Dimitrios Apostolou

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).