Re: [PATCH][alias-improvements] Fix PR38721 - ensure gimple_addressable_vars is up-to-date

[Richard Guenther <rguenther at suse dot de>]

On Wed, 7 Jan 2009, Richard Guenther wrote:

> 
> PR38721 is a case where we have the address taken of a local variable
> but that variable is not included in gimple_addressable_vars.  This is
> bad, because the alias oracle relies on this information to be corrrect
> and in the failure case considers a variable non-aliased.
> 
> This patch introduces TODO_update_address_taken (that was on my TODO-list
> for some time) and invokes it from inlining (and function versioning) as
> well as the vectorizer and IVOPTs which are the only passes introducing
> new addresses of something that didn't have its address taken before.
> 
> On a general remark - like for the call-clobber bitmap, addressable-vars
> is only required for local variables (it currently also contains globals).
> On the branch it should now be possible to install the very old
> introduce-a-global-uid-to-decl-mapping-and-make-referenced_vars-a-bitmap
> patches.  Even for referenced-vars we should be fine with local
> variables only.
> 
> Well.  This patch makes sure addressable-vars is correct during the time
> we are in SSA form.
> 
> I'm currently re-bootstrapping and testing this on 
> x86_64-unknown-linux-gnu due to some cosmetic changes and will install it
> on the branch afterwards.
> 
> Comments always welcome (I am trying to not do too much followup cleanup
> work on the branch - it is already big enough - but plan to work on
> trunk once stage1 opens).

This is what I ended up installing.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

Richard.

2009-01-07  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/38721
	* tree-into-ssa.c (pass_build_ssa): Add TODO_update_address_taken.
	* tree-pass.h (TODO_update_address_taken): New flag.
	* tree-ssa-loop.c (tree_ssa_loop_ivopts): Update address-taken.
	* tree-ssa.c (execute_update_addresses_taken): Optimize only when
	optimizing.
	(pass_update_address_taken): Just use TODO_update_address_taken.
	* tree-flow.h (execute_update_addresses_taken): Update prototype.
	* tree-cfg.c (verify_expr): Verify that stmt addresses-taken and
	function addressable-vars are conservatively correct.
	(verify_stmt): Initialize gsi of walk data.
	* tree-inline.c (optimize_inline_calls): Execute
	TODO_update_address_taken.
	(tree_function_versioning): Call execute_update_addresses_taken.
	* passes.c (execute_function_todo): Handle TODO_update_address_taken.
	(init_optimization_passes): Remove redundant update-address-taken pass
	after final inlining.
	* tree-parloops.c (parallelize_loops): Call
	execute_update_addresses_taken.
	* tree-vectorizer.c (vectorize_loops): Likewise.


Index: gcc/tree-into-ssa.c
===================================================================
*** gcc/tree-into-ssa.c	(revision 143152)
--- gcc/tree-into-ssa.c	(working copy)
*************** struct gimple_opt_pass pass_build_ssa =
*** 2304,2310 ****
    PROP_ssa,				/* properties_provided */
    0,					/* properties_destroyed */
    0,					/* todo_flags_start */
!   TODO_dump_func
      | TODO_update_ssa_only_virtuals
      | TODO_verify_ssa
      | TODO_remove_unused_locals		/* todo_flags_finish */
--- 2304,2311 ----
    PROP_ssa,				/* properties_provided */
    0,					/* properties_destroyed */
    0,					/* todo_flags_start */
!   TODO_update_address_taken
!     | TODO_dump_func
      | TODO_update_ssa_only_virtuals
      | TODO_verify_ssa
      | TODO_remove_unused_locals		/* todo_flags_finish */
Index: gcc/tree-pass.h
===================================================================
*** gcc/tree-pass.h	(revision 143152)
--- gcc/tree-pass.h	(working copy)
*************** struct dump_file_info
*** 285,291 ****
  #define TODO_mark_first_instance	(1 << 19)
  
  /* Rebuild aliasing info.  */
! #define TODO_rebuild_alias                (1 << 20)
  
  #define TODO_update_ssa_any		\
      (TODO_update_ssa			\
--- 285,294 ----
  #define TODO_mark_first_instance	(1 << 19)
  
  /* Rebuild aliasing info.  */
! #define TODO_rebuild_alias              (1 << 20)
! 
! /* Rebuild the addressable-vars bitmap and do register promotion.  */
! #define TODO_update_address_taken	(1 << 21)
  
  #define TODO_update_ssa_any		\
      (TODO_update_ssa			\
Index: gcc/tree-ssa-loop.c
===================================================================
*** gcc/tree-ssa-loop.c	(revision 143152)
--- gcc/tree-ssa-loop.c	(working copy)
*************** tree_ssa_loop_ivopts (void)
*** 665,671 ****
      return 0;
  
    tree_ssa_iv_optimize ();
!   return 0;
  }
  
  static bool
--- 665,674 ----
      return 0;
  
    tree_ssa_iv_optimize ();
!   /* ???  We sometimes use TMR_BASE with an invariant ADDR_EXPR instead
!      of using TMR_SYMBOL.  This makes variables having their address
!      taken (libgcc/config/libbid/bid128_fma.c).  */
!   return TODO_update_address_taken;
  }
  
  static bool
Index: gcc/tree-parloops.c
===================================================================
*** gcc/tree-parloops.c	(revision 143152)
--- gcc/tree-parloops.c	(working copy)
*************** parallelize_loops (void)
*** 1866,1871 ****
--- 1866,1875 ----
  
        changed = true;
        gen_parallel_loop (loop, reduction_list, n_threads, &niter_desc);
+       /* ???  If verify_loop_closed_ssa would not call verify_ssa or that
+          would not call verify_stmts we could defer this to after processing
+ 	 all loops.  */
+       execute_update_addresses_taken (false);
        verify_flow_info ();
        verify_dominators (CDI_DOMINATORS);
        verify_loop_structure ();
Index: gcc/tree-vectorizer.c
===================================================================
*** gcc/tree-vectorizer.c	(revision 143152)
--- gcc/tree-vectorizer.c	(working copy)
*************** vectorize_loops (void)
*** 2818,2823 ****
--- 2818,2827 ----
  	  continue;
  
  	vect_transform_loop (loop_vinfo);
+ 	/* ???  If verify_loop_closed_ssa would not call verify_ssa or that
+ 	   would not call verify_stmts we could defer this to after processing
+ 	   all loops.  */
+ 	execute_update_addresses_taken (false);
  	num_vectorized_loops++;
        }
    vect_loop_location = UNKNOWN_LOC;
*************** vectorize_loops (void)
*** 2847,2853 ****
  
    free_stmt_vec_info_vec ();
  
!   return num_vectorized_loops > 0 ? TODO_cleanup_cfg : 0;
  }
  
  /* Increase alignment of global arrays to improve vectorization potential.
--- 2851,2858 ----
  
    free_stmt_vec_info_vec ();
  
!   return (num_vectorized_loops > 0
! 	  ? TODO_cleanup_cfg : 0);
  }
  
  /* Increase alignment of global arrays to improve vectorization potential.
Index: gcc/tree-ssa.c
===================================================================
*** gcc/tree-ssa.c	(revision 143152)
--- gcc/tree-ssa.c	(working copy)
*************** struct gimple_opt_pass pass_late_warn_un
*** 1469,1476 ****
  
  /* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables.  */
  
! unsigned int
! execute_update_addresses_taken (void)
  {
    tree var;
    referenced_var_iterator rvi;
--- 1469,1476 ----
  
  /* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables.  */
  
! void
! execute_update_addresses_taken (bool do_optimize)
  {
    tree var;
    referenced_var_iterator rvi;
*************** execute_update_addresses_taken (void)
*** 1530,1572 ****
  
    /* When possible, clear ADDRESSABLE bit or set the REGISTER bit
       and mark variable for conversion into SSA.  */
!   FOR_EACH_REFERENCED_VAR (var, rvi)
!     {
!       /* Global Variables, result decls cannot be changed.  */
!       if (is_global_var (var)
!           || TREE_CODE (var) == RESULT_DECL
! 	  || bitmap_bit_p (addresses_taken, DECL_UID (var)))
! 	continue;
! 	
!       if (TREE_ADDRESSABLE (var))
! 	{
! 	  clear_call_clobbered (var);
! 	  TREE_ADDRESSABLE (var) = 0;
! 	  if (is_gimple_reg (var))
  	    mark_sym_for_renaming (var);
! 	  update_vops = true;
! 	  if (dump_file)
! 	    {
! 	      fprintf (dump_file, "No longer having address taken ");
! 	      print_generic_expr (dump_file, var, 0);
! 	      fprintf (dump_file, "\n");
! 	    }
! 	}
!       if (!DECL_GIMPLE_REG_P (var)
! 	  && !bitmap_bit_p (not_reg_needs, DECL_UID (var))
! 	  && (TREE_CODE (TREE_TYPE (var)) == COMPLEX_TYPE
! 	      || TREE_CODE (TREE_TYPE (var)) == VECTOR_TYPE))
! 	{
! 	  DECL_GIMPLE_REG_P (var) = 1;
! 	  mark_sym_for_renaming (var);
! 	  update_vops = true;
! 	  if (dump_file)
! 	    {
! 	      fprintf (dump_file, "Decl is now a gimple register ");
! 	      print_generic_expr (dump_file, var, 0);
! 	      fprintf (dump_file, "\n");
! 	    }
! 	}
        }
  
    /* Operand caches needs to be recomputed for operands referencing the updated
--- 1530,1573 ----
  
    /* When possible, clear ADDRESSABLE bit or set the REGISTER bit
       and mark variable for conversion into SSA.  */
!   if (optimize && do_optimize)
!     FOR_EACH_REFERENCED_VAR (var, rvi)
!       {
! 	/* Global Variables, result decls cannot be changed.  */
! 	if (is_global_var (var)
! 	    || TREE_CODE (var) == RESULT_DECL
! 	    || bitmap_bit_p (addresses_taken, DECL_UID (var)))
! 	  continue;
! 
! 	if (TREE_ADDRESSABLE (var))
! 	  {
! 	    clear_call_clobbered (var);
! 	    TREE_ADDRESSABLE (var) = 0;
! 	    if (is_gimple_reg (var))
! 	      mark_sym_for_renaming (var);
! 	    update_vops = true;
! 	    if (dump_file)
! 	      {
! 		fprintf (dump_file, "No longer having address taken ");
! 		print_generic_expr (dump_file, var, 0);
! 		fprintf (dump_file, "\n");
! 	      }
! 	  }
! 	if (!DECL_GIMPLE_REG_P (var)
! 	    && !bitmap_bit_p (not_reg_needs, DECL_UID (var))
! 	    && (TREE_CODE (TREE_TYPE (var)) == COMPLEX_TYPE
! 		|| TREE_CODE (TREE_TYPE (var)) == VECTOR_TYPE))
! 	  {
! 	    DECL_GIMPLE_REG_P (var) = 1;
  	    mark_sym_for_renaming (var);
! 	    update_vops = true;
! 	    if (dump_file)
! 	      {
! 		fprintf (dump_file, "Decl is now a gimple register ");
! 		print_generic_expr (dump_file, var, 0);
! 		fprintf (dump_file, "\n");
! 	      }
! 	  }
        }
  
    /* Operand caches needs to be recomputed for operands referencing the updated
*************** execute_update_addresses_taken (void)
*** 1587,1594 ****
      }
  
    BITMAP_FREE (not_reg_needs);
- 
-   return 0;
  }
  
  struct gimple_opt_pass pass_update_address_taken =
--- 1588,1593 ----
*************** struct gimple_opt_pass pass_update_addre
*** 1597,1603 ****
    GIMPLE_PASS,
    "addressables",			/* name */
    NULL,					/* gate */
!   execute_update_addresses_taken,	/* execute */
    NULL,					/* sub */
    NULL,					/* next */
    0,					/* static_pass_number */
--- 1596,1602 ----
    GIMPLE_PASS,
    "addressables",			/* name */
    NULL,					/* gate */
!   NULL,					/* execute */
    NULL,					/* sub */
    NULL,					/* next */
    0,					/* static_pass_number */
*************** struct gimple_opt_pass pass_update_addre
*** 1606,1611 ****
    0,					/* properties_provided */
    0,					/* properties_destroyed */
    0,					/* todo_flags_start */
!   TODO_dump_func			/* todo_flags_finish */
   }
  };
--- 1605,1611 ----
    0,					/* properties_provided */
    0,					/* properties_destroyed */
    0,					/* todo_flags_start */
!   TODO_update_address_taken
!   | TODO_dump_func			/* todo_flags_finish */
   }
  };
Index: gcc/tree-inline.c
===================================================================
*** gcc/tree-inline.c	(revision 143152)
--- gcc/tree-inline.c	(working copy)
*************** optimize_inline_calls (tree fn)
*** 3570,3575 ****
--- 3570,3576 ----
    return (TODO_update_ssa
  	  | TODO_cleanup_cfg
  	  | (gimple_in_ssa_p (cfun) ? TODO_remove_unused_locals : 0)
+ 	  | (gimple_in_ssa_p (cfun) ? TODO_update_address_taken : 0)
  	  | (profile_status != PROFILE_ABSENT ? TODO_rebuild_frequencies : 0));
  }
  
*************** tree_function_versioning (tree old_decl,
*** 4363,4368 ****
--- 4364,4370 ----
  	  if (need_ssa_update_p ())
  	    update_ssa (TODO_update_ssa);
  	}
+       execute_update_addresses_taken (false);
      }
    free_dominance_info (CDI_DOMINATORS);
    free_dominance_info (CDI_POST_DOMINATORS);
Index: gcc/tree-flow.h
===================================================================
*** gcc/tree-flow.h	(revision 143152)
--- gcc/tree-flow.h	(working copy)
*************** extern void flush_pending_stmts (edge);
*** 719,725 ****
  extern void verify_ssa (bool);
  extern void delete_tree_ssa (void);
  extern bool ssa_undefined_value_p (tree);
! unsigned int execute_update_addresses_taken (void);
  
  /* Call-back function for walk_use_def_chains().  At each reaching
     definition, a function with this prototype is called.  */
--- 719,725 ----
  extern void verify_ssa (bool);
  extern void delete_tree_ssa (void);
  extern bool ssa_undefined_value_p (tree);
! extern void execute_update_addresses_taken (bool);
  
  /* Call-back function for walk_use_def_chains().  At each reaching
     definition, a function with this prototype is called.  */
Index: gcc/tree-cfg.c
===================================================================
*** gcc/tree-cfg.c	(revision 143152)
--- gcc/tree-cfg.c	(working copy)
*************** gimple_split_edge (edge edge_in)
*** 2783,2790 ****
     inside a PHI node.  */
  
  static tree
! verify_expr (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
  {
    tree t = *tp, x;
  
    if (TYPE_P (t))
--- 2783,2791 ----
     inside a PHI node.  */
  
  static tree
! verify_expr (tree *tp, int *walk_subtrees, void *data)
  {
+   struct walk_stmt_info *wi = (struct walk_stmt_info *)data;
    tree t = *tp, x;
  
    if (TYPE_P (t))
*************** verify_expr (tree *tp, int *walk_subtree
*** 2862,2868 ****
  	     x = TREE_OPERAND (x, 0))
  	  ;
  
! 	if (TREE_CODE (x) != VAR_DECL && TREE_CODE (x) != PARM_DECL)
  	  return NULL;
  	if (!TREE_ADDRESSABLE (x))
  	  {
--- 2863,2871 ----
  	     x = TREE_OPERAND (x, 0))
  	  ;
  
! 	if (!(TREE_CODE (x) == VAR_DECL
! 	      || TREE_CODE (x) == PARM_DECL
! 	      || TREE_CODE (x) == RESULT_DECL))
  	  return NULL;
  	if (!TREE_ADDRESSABLE (x))
  	  {
*************** verify_expr (tree *tp, int *walk_subtree
*** 2870,2875 ****
--- 2873,2897 ----
  	    return x;
  	  }
  
+ 	if (!gimple_in_ssa_p (cfun))
+ 	  break;
+ 
+ 	/* Make sure the stmt addresses_taken bitmap is conservatively
+ 	   correct.  Likewise the per-function addressable_vars bitmap.  */
+ 	if (!bitmap_bit_p (gimple_addresses_taken (gsi_stmt (wi->gsi)),
+ 			   DECL_UID (x)))
+ 	  {
+ 	    error ("address taken, but stmt addresses_taken bitmap not "
+ 		   "up-to-date");
+ 	    return x;
+ 	  }
+ 	if (!is_global_var (x)
+ 	    && !bitmap_bit_p (gimple_addressable_vars (cfun), DECL_UID (x)))
+ 	  {
+ 	    error ("address taken, but decl not included in addressable-vars");
+ 	    return x;
+ 	  }
+ 
  	break;
        }
  
*************** verify_stmt (gimple_stmt_iterator *gsi)
*** 4053,4058 ****
--- 4075,4081 ----
      }
  
    memset (&wi, 0, sizeof (wi));
+   wi.gsi = *gsi;
    addr = walk_gimple_op (gsi_stmt (*gsi), verify_expr, &wi);
    if (addr)
      {
Index: gcc/passes.c
===================================================================
*** gcc/passes.c	(revision 143152)
--- gcc/passes.c	(working copy)
*************** init_optimization_passes (void)
*** 586,592 ****
        /* Initial scalar cleanups before alias computation.
  	 They ensure memory accesses are not indirect wherever possible.  */
        NEXT_PASS (pass_strip_predict_hints);
-       NEXT_PASS (pass_update_address_taken);
        NEXT_PASS (pass_rename_ssa_copies);
        NEXT_PASS (pass_complete_unrolli);
        NEXT_PASS (pass_ccp);
--- 586,591 ----
*************** execute_function_todo (void *data)
*** 941,949 ****
        cfun->last_verified &= ~TODO_verify_ssa;
      }
    
    if (flags & TODO_rebuild_alias)
      {
!       execute_update_addresses_taken ();
        compute_may_aliases ();
        cfun->curr_properties |= PROP_alias;
      }
--- 940,952 ----
        cfun->last_verified &= ~TODO_verify_ssa;
      }
    
+   if (flags & TODO_update_address_taken)
+     execute_update_addresses_taken (true);
+ 
    if (flags & TODO_rebuild_alias)
      {
!       if (!(flags & TODO_update_address_taken))
! 	execute_update_addresses_taken (true);
        compute_may_aliases ();
        cfun->curr_properties |= PROP_alias;
      }