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

login
register
mail settings
Submitter Alexandre Oliva
Date Jan. 20, 2011, 9:27 p.m.
Message ID <oripxje04y.fsf@livre.localdomain>
Download mbox | patch
Permalink /patch/79771/
State New
Headers show

Comments

Alexandre Oliva - Jan. 20, 2011, 9:27 p.m.
On Jan 10, 2011, Richard Guenther <richard.guenther@gmail.com> wrote:

> I guess I'm more happy with the patch if you switch the
> initialization in create_var_ann to mark the variable used.

Like this?
Richard Guenther - Jan. 21, 2011, 10:35 a.m.
On Thu, Jan 20, 2011 at 10:27 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Jan 10, 2011, Richard Guenther <richard.guenther@gmail.com> wrote:
>
>> I guess I'm more happy with the patch if you switch the
>> initialization in create_var_ann to mark the variable used.
>
> Like this?

Yes.  This is ok if it passes bootstap & regtest.

Thanks,
Richard.

>
>
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer
>
>
H.J. Lu - Jan. 21, 2011, 6:55 p.m.
On Fri, Jan 21, 2011 at 2:35 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, Jan 20, 2011 at 10:27 PM, Alexandre Oliva <aoliva@redhat.com> wrote:
>> On Jan 10, 2011, Richard Guenther <richard.guenther@gmail.com> wrote:
>>
>>> I guess I'm more happy with the patch if you switch the
>>> initialization in create_var_ann to mark the variable used.
>>
>> Like this?
>
> Yes.  This is ok if it passes bootstap & regtest.
>

Alexandre, did you run bootstap before checking in your
change? It caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47402
Alexandre Oliva - Jan. 21, 2011, 9:55 p.m.
On Jan 21, 2011, "H.J. Lu" <hjl.tools@gmail.com> wrote:

> Alexandre, did you run bootstap before checking in your
> change?

Yep.  According to my records, I ran it on top of revision 169058.
However, my subsequent bootstrap, with revision 169093 (pristine),
showed a -fcompare-debug regression in fortran/module.o.  Very odd...
Looking into it.
Alexandre Oliva - Jan. 21, 2011, 10:40 p.m.
On Jan 20, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:

> 	* tree-dfa.c (create_var_ann): Mark variable as used.

> Index: gcc/tree-dfa.c
> ===================================================================
> --- gcc/tree-dfa.c.orig	2011-01-20 15:42:18.837908738 -0200
> +++ gcc/tree-dfa.c	2011-01-20 15:44:10.794077945 -0200
> @@ -137,6 +137,9 @@ create_var_ann (tree t)
>    ann = ggc_alloc_cleared_var_ann_d ();
>    *DECL_VAR_ANN_PTR (t) = ann;
 
> +  /* Assume the variable is used, at least for now.  */
> +  ann->used = true;
> +
>    return ann;
>  }
 
This hunk caused PR 47402, so I've reverted it, but I left the rest of
the patch in.  I'll be away tomorrow, but I'll look back into this on
Sunday to figure out why we're making different decisions regarding the
(partial) inlining of get_integer, and their different stack sizes.  I
suspect we may have to propagate used flags when cloning variables, or
run some pass to figure out what's used/unused after (partial) cloning,
for we seem to be looking at default-initialized (not recomputed)
information, a possibility that IIUC you and honza were concered about
when reviewing the original patch, but that went way over the top of my
head :-(

As I said, I'll look into it on Sunday, but any tips on where to insert
recomputation or somesuch would be highly appreciated.

TIA,

Patch

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR debug/47106
	* cfgexpand.c (account_used_vars_for_block): Only account vars
	that are annotated as used.
	(estimated_stack_frame_size): Don't set TREE_USED.
	* tree-dfa.c (create_var_ann): Mark variable as used.

Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig	2011-01-20 14:55:54.405562397 -0200
+++ gcc/cfgexpand.c	2011-01-20 15:18:29.628014702 -0200
@@ -1325,7 +1325,7 @@  account_used_vars_for_block (tree block,
 
   /* Expand all variables at this level.  */
   for (t = BLOCK_VARS (block); t ; t = DECL_CHAIN (t))
-    if (TREE_USED (t))
+    if (var_ann (t) && var_ann (t)->used)
       size += expand_one_var (t, toplevel, false);
 
   /* Expand all variables at containing levels.  */
@@ -1389,9 +1389,10 @@  estimated_stack_frame_size (tree decl)
 
   FOR_EACH_LOCAL_DECL (cfun, ix, var)
     {
+      /* TREE_USED marks local variables that do not appear in lexical
+	 blocks.  We don't want to expand those that do twice.  */
       if (TREE_USED (var))
         size += expand_one_var (var, true, false);
-      TREE_USED (var) = 1;
     }
   size += account_used_vars_for_block (outer_block, true);
 
Index: gcc/tree-dfa.c
===================================================================
--- gcc/tree-dfa.c.orig	2011-01-20 15:42:18.837908738 -0200
+++ gcc/tree-dfa.c	2011-01-20 15:44:10.794077945 -0200
@@ -137,6 +137,9 @@  create_var_ann (tree t)
   ann = ggc_alloc_cleared_var_ann_d ();
   *DECL_VAR_ANN_PTR (t) = ann;
 
+  /* Assume the variable is used, at least for now.  */
+  ann->used = true;
+
   return ann;
 }