diff mbox

[PR,debug/47106] account used vars only once

Message ID ord3n8tf2m.fsf@livre.localdomain
State New
Headers show

Commit Message

Alexandre Oliva Feb. 4, 2011, 7:49 a.m. UTC 
On Feb  3, 2011, Richard Guenther <richard.guenther@gmail.com> wrote:

> On Thu, Feb 3, 2011 at 6:50 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
>> On Feb  2, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:
>> 
>>> Perhaps relying on the used flag and going over all scope blocks wasn't
>>> such a good idea, after all; I suppose we could get something more
>>> reliable using referenced_vars only.  Although I'm finding it difficult
>>> to figure out whether presence in referenced_vars should ever be
>>> different from having the used flag marked, it appears that presence in
>>> referenced_vars is maintained more accurately, even during inlining.
>> 
>> Indeed, early in compilation we don't have referenced_vars at all, but
>> we can make do without them, going through the scope blocks.

> I think we can simply move pass_inline_parameters to after
> pass_referenced_vars.

That isn't enough.  We often attempt to estimate the stack size of a
function before we as much as put it in SSA form (let alone compute
referenced_vras), while processing other functions.

> referenced_var_lookup_in should rather take a struct function argument
> than a hashtable (in fact, given the few existing callers I'd just change
> the referenced_var_lookup signature).

*nod*

> Can you do a quick comparison between the old and the new
> stack frame estimations on some random files from gcc?

I guess...  Once we have another working patch.  My previous patch did
away with the need for rearranging passes by using the current
computation when referenced_vars are not available, but if we switch to
referenced_vars only, we have to figure out how to rearrange the passes
so that we make reasonable estimates.

Here's where I stand ATM.  The first patch is supposed to implement the
changes you suggested, plus some attempts to fix things up or otherwise
detect problems.  It finds too many.  The second patch restores some of
the removed functionality to error out if the stack size computations
differ.  None of these are meant for inclusion, I post them for the
record in case any of you guys would like to pick it up while I get some
sleep.  Suggestions on how to proceed are welcome.  I'm thinking of
rearranging the passes so we compute referenced_vars for all functions
before advancing to other passes for any other functions, but I have no
idea of how to do that.  I'll dig it up if nobody beats me to it.
diff mbox

Patch

Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig	2011-02-04 05:26:40.137008650 -0200
+++ gcc/cfgexpand.c	2011-02-04 05:27:40.951213575 -0200
@@ -1311,6 +1311,29 @@  create_stack_guard (void)
   crtl->stack_protect_guard = guard;
 }
 
+/* A subroutine of expand_used_vars.  Walk down through the BLOCK tree
+   expanding variables.  Those variables that can be put into registers
+   are allocated pseudos; those that can't are put on the stack.
+
+   TOPLEVEL is true if this is the outermost BLOCK.  */
+
+static HOST_WIDE_INT
+account_used_vars_for_block (tree block, bool toplevel)
+{
+  tree t;
+  HOST_WIDE_INT size = 0;
+
+  /* Expand all variables at this level.  */
+  for (t = BLOCK_VARS (block); t ; t = DECL_CHAIN (t))
+    size += expand_one_var (t, toplevel, false);
+
+  /* Expand all variables at containing levels.  */
+  for (t = BLOCK_SUBBLOCKS (block); t ; t = BLOCK_CHAIN (t))
+    size += account_used_vars_for_block (t, false);
+
+  return size;
+}
+
 /* Prepare for expanding variables.  */
 static void
 init_vars_expansion (void)
@@ -1354,7 +1377,10 @@  HOST_WIDE_INT
 estimated_stack_frame_size (tree decl)
 {
   HOST_WIDE_INT size = 0;
+  HOST_WIDE_INT osize = 0;
+  unsigned ix;
   size_t i;
+  tree outer_block = DECL_INITIAL (current_function_decl);
   tree var;
   tree old_cur_fun_decl = current_function_decl;
   referenced_var_iterator rvi;
@@ -1378,6 +1404,11 @@  estimated_stack_frame_size (tree decl)
       size += account_stack_vars ();
       fini_vars_expansion ();
     }
+
+  if (size != osize)
+    error ("est stack size changed from %li to %li",
+	   (long)osize, (long)size);
+
   pop_cfun ();
   current_function_decl = old_cur_fun_decl;
   return size;