This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [tuples] RFC - create bind_expr_stack and temp_htab lazily


On Wed, 9 Jul 2008, Jakub Jelinek wrote:

Hi!

push_gimplify_context/pop_gimplify_context is called quite frequently,
especially by force_gimple_operand, so IMHO it pays off to have
it fast and avoid unnecessary memory allocations (especially GC ones).
I've gathered some data for these with libgcc, libstdc++-v3 build
+ insn-attrtab.c + 2 random big C++ testcases.
From 25553 push_gimplify_context/pop_gimplify_context pairs
in 17633 cases (i.e. 69%) bind_expr_stack wasn't touched at all and so
the allocation of push_gimplify_context just wasted some GC memory.
gimple_push_bind_expr was caled 12650 times and in the case of lazy
allocation below that costs us on x86_64 just a predicted not taken
testq %rdi, %rdi; je something.
Similarly, out of the same 25553 pairs in 13279 (i.e. 52%) of cases
temp_htab wasn't touched at all, which means unnecessary temp_htab
creation (two xcalloc calls, two free calls).  In this case
lookup_tmp_var accessed temp_htab 120234 times, which is more than
in the bind_expr_stack case, but gimplify_ctxp had to be loaded in
register anyway (to be passed to htab_find_slot), so again it
is just a cost of a testq and je, and IMHO even 120000 testq/je
when the register is mostly non-NULL are cheaper than 13279 unnecessary
htab_create/htab_delete calls.

Is this ok for tuples (or trunk), or should I gather more statistics?

BTW, as I already talked about on IRC, given that push_gimplify_context
and pop_gimplify_context are always called in pairs within the same
functions, we could save even the remaining xcalloc in
push_gimplify_context, by passing address of an automatic gimplify_ctx
variable to push_gimplify_context (or by using pthread_cleanup_{push,pop}
like macros where one starts with { and the other has the corresponding
closing } and the local variable is in that block).  The disadvantage
is that gimplify_ctx type, which is currently private to gimplify.c, would
need to be in a header file (tree-gimple.h?).

IMHO the patch below is ok for trunk. The macro thing is ugly, but providing the storage from the caller would be ok.


Richard.


2008-07-09 Jakub Jelinek <jakub@redhat.com>


	* gimplify.c (push_gimplify_context): Don't allocate bind_expr_stack
	and temp_htab here.
	(pop_gimplify_context): Allow bind_expr_stack being NULL.  Check
	c->temp_htab instead of optimize whether htab_delete should be called.
	(gimple_push_bind_expr): Create bind_expr_stack lazily.
	(lookup_tmp_var): Create temp_htab lazily.

--- gcc/gimplify.c.jj	2008-07-04 16:32:58.000000000 +0200
+++ gcc/gimplify.c	2008-07-09 14:10:08.000000000 +0200
@@ -220,9 +220,6 @@ push_gimplify_context (void)

  c = (struct gimplify_ctx *) xcalloc (1, sizeof (struct gimplify_ctx));
  c->prev_context = gimplify_ctxp;
-  c->bind_expr_stack = VEC_alloc (gimple, heap, 8);
-  if (optimize)
-    c->temp_htab = htab_create (1000, gimple_tree_hash, gimple_tree_eq, free);

  gimplify_ctxp = c;
}
@@ -239,7 +236,8 @@ pop_gimplify_context (gimple body)
  struct gimplify_ctx *c = gimplify_ctxp;
  tree t;

-  gcc_assert (c && VEC_empty (gimple, c->bind_expr_stack));
+  gcc_assert (c && (c->bind_expr_stack == NULL
+		    || VEC_empty (gimple, c->bind_expr_stack)));
  gimplify_ctxp = c->prev_context;

  for (t = c->temps; t ; t = TREE_CHAIN (t))
@@ -250,7 +248,7 @@ pop_gimplify_context (gimple body)
  else
    record_vars (c->temps);

-  if (optimize)
+  if (c->temp_htab)
    htab_delete (c->temp_htab);
  free (c);
}
@@ -258,6 +256,8 @@ pop_gimplify_context (gimple body)
static void
gimple_push_bind_expr (gimple gimple_bind)
{
+  if (gimplify_ctxp->bind_expr_stack == NULL)
+    gimplify_ctxp->bind_expr_stack = VEC_alloc (gimple, heap, 8);
  VEC_safe_push (gimple, heap, gimplify_ctxp->bind_expr_stack, gimple_bind);
}

@@ -611,6 +611,9 @@ lookup_tmp_var (tree val, bool is_formal
      void **slot;

      elt.val = val;
+      if (gimplify_ctxp->temp_htab == NULL)
+        gimplify_ctxp->temp_htab
+	  = htab_create (1000, gimple_tree_hash, gimple_tree_eq, free);
      slot = htab_find_slot (gimplify_ctxp->temp_htab, (void *)&elt, INSERT);
      if (*slot == NULL)
	{

Jakub



-- Richard Guenther <rguenther@suse.de> Novell / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 - GF: Markus Rex


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]