diff mbox

Tidy up execute_update_addresses_taken

Message ID 201009222335.18725.ebotcazou@adacore.com
State New
Headers show

Commit Message

Eric Botcazou Sept. 22, 2010, 9:35 p.m. UTC 
Hi,

I happened to notice that execute_update_addresses_taken is only called with 
DO_OPTIMIZE set to true and when optimization is enabled, so the apparently 
weird behaviour of the function (doing a complicated analysis and dropping 
the result half of the time) is only apparent.  The attached patch cleans 
this up a little.  Tested on i586-suse-linux, OK for mainline?


2010-09-22  Eric Botcazou  <ebotcazou@adacore.com>

	* tree-flow.h (execute_update_addresses_taken): Adjust.
	* tree-ssa.c (execute_update_addresses_taken): Remove parameter and
	test of OPTIMIZE.
	* passes.c (execute_function_todo): Adjust calls to above function.

Comments

Richard Biener Sept. 22, 2010, 9:55 p.m. UTC | #1 
On Wed, Sep 22, 2010 at 11:35 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> I happened to notice that execute_update_addresses_taken is only called with
> DO_OPTIMIZE set to true and when optimization is enabled, so the apparently
> weird behaviour of the function (doing a complicated analysis and dropping
> the result half of the time) is only apparent.  The attached patch cleans
> this up a little.  Tested on i586-suse-linux, OK for mainline?

Huh, interesting ;)  I wonder when we dropped the last caller with
false or !optimize ...

Well, the patch is ok anyway.

Thanks,
Richard.

>
> 2010-09-22  Eric Botcazou  <ebotcazou@adacore.com>
>
>        * tree-flow.h (execute_update_addresses_taken): Adjust.
>        * tree-ssa.c (execute_update_addresses_taken): Remove parameter and
>        test of OPTIMIZE.
>        * passes.c (execute_function_todo): Adjust calls to above function.
>
>
> --
> Eric Botcazou
>
Eric Botcazou Sept. 23, 2010, 6:28 a.m. UTC | #2 
> Huh, interesting ;)  I wonder when we dropped the last caller with
> false or !optimize ...

You (re-)introduced one yesterday. :-)  So the patch must be adjusted.  What 
about calling execute_update_addresses_taken from execute_function_todo only 
if OPTIMIZE is true?  That would avoid running the collecting phase for 
nothing.  Or else early returning from the function if !OPTIMIZE?
Richard Biener Sept. 23, 2010, 10 a.m. UTC | #3 
On Thu, Sep 23, 2010 at 8:28 AM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> Huh, interesting ;)  I wonder when we dropped the last caller with
>> false or !optimize ...
>
> You (re-)introduced one yesterday. :-)  So the patch must be adjusted.  What
> about calling execute_update_addresses_taken from execute_function_todo only
> if OPTIMIZE is true?  That would avoid running the collecting phase for
> nothing.  Or else early returning from the function if !OPTIMIZE?

Not calling it from execute_function_todo makes sense - it doesn't make
any sense to compute things but then to do nothing.

Thanks,
Richard.

> --
> Eric Botcazou
>
diff mbox

Patch

Index: tree-ssa.c
===================================================================
--- tree-ssa.c	(revision 164507)
+++ tree-ssa.c	(working copy)
@@ -1944,10 +1944,11 @@  maybe_optimize_var (tree var, bitmap add
   return update_vops;
 }
 
-/* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables.  */
+/* When possible, clear TREE_ADDRESSABLE bit or set DECL_GIMPLE_REG_P bit for
+   local variables.  */
 
 void
-execute_update_addresses_taken (bool do_optimize)
+execute_update_addresses_taken (void)
 {
   tree var;
   gimple_stmt_iterator gsi;
@@ -2047,18 +2048,13 @@  execute_update_addresses_taken (bool do_
 	}
     }
 
-  /* When possible, clear ADDRESSABLE bit or set the REGISTER bit
-     and mark variable for conversion into SSA.  */
-  if (optimize && do_optimize)
-    {
-      /* We cannot iterate over all referenced vars as that can contain
-	 unused vars from BLOCK trees which cause code generation
-	 differences for -g vs. -g0.  */
-      for (var = DECL_ARGUMENTS (cfun->decl); var; var = DECL_CHAIN (var))
-	update_vops |= maybe_optimize_var (var, addresses_taken, not_reg_needs);
-      FOR_EACH_VEC_ELT (tree, cfun->local_decls, i, var)
-	update_vops |= maybe_optimize_var (var, addresses_taken, not_reg_needs);
-    }
+  /* We cannot iterate over all referenced vars because that can contain
+     unused vars from BLOCK trees, which causes code generation differences
+     for -g vs. -g0.  */
+  for (var = DECL_ARGUMENTS (cfun->decl); var; var = DECL_CHAIN (var))
+    update_vops |= maybe_optimize_var (var, addresses_taken, not_reg_needs);
+  FOR_EACH_VEC_ELT (tree, cfun->local_decls, i, var)
+    update_vops |= maybe_optimize_var (var, addresses_taken, not_reg_needs);
 
   /* Operand caches needs to be recomputed for operands referencing the updated
      variables.  */
Index: tree-flow.h
===================================================================
--- tree-flow.h	(revision 164507)
+++ tree-flow.h	(working copy)
@@ -559,7 +559,7 @@  extern void delete_tree_ssa (void);
 extern bool ssa_undefined_value_p (tree);
 extern void warn_uninit (tree, const char *, void *);
 extern unsigned int warn_uninitialized_vars (bool);
-extern void execute_update_addresses_taken (bool);
+extern void execute_update_addresses_taken (void);
 
 /* Call-back function for walk_use_def_chains().  At each reaching
    definition, a function with this prototype is called.  */
Index: passes.c
===================================================================
--- passes.c	(revision 164507)
+++ passes.c	(working copy)
@@ -1209,12 +1209,12 @@  execute_function_todo (void *data)
     }
 
   if (flags & TODO_update_address_taken)
-    execute_update_addresses_taken (true);
+    execute_update_addresses_taken ();
 
   if (flags & TODO_rebuild_alias)
     {
       if (!(flags & TODO_update_address_taken))
-	execute_update_addresses_taken (true);
+	execute_update_addresses_taken ();
       compute_may_aliases ();
     }