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]

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


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).

Thanks,
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-vectorizer.c (vectorize_loops): Likewise.
	* 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.

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-vectorizer.c
===================================================================
*** gcc/tree-vectorizer.c	(revision 143152)
--- gcc/tree-vectorizer.c	(working copy)
*************** 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.
--- 2847,2854 ----
  
    free_stmt_vec_info_vec ();
  
!   return (num_vectorized_loops > 0
! 	  ? TODO_cleanup_cfg | TODO_update_address_taken : 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,1475 ****
  
  /* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables.  */
  
! unsigned int
  execute_update_addresses_taken (void)
  {
    tree var;
--- 1469,1475 ----
  
  /* Compute TREE_ADDRESSABLE and DECL_GIMPLE_REG_P for local variables.  */
  
! void
  execute_update_addresses_taken (void)
  {
    tree var;
*************** 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)
!     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 ();
      }
    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 (void);
  
  /* 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 ();
+ 
    if (flags & TODO_rebuild_alias)
      {
!       if (!(flags & TODO_update_address_taken))
! 	execute_update_addresses_taken ();
        compute_may_aliases ();
        cfun->curr_properties |= PROP_alias;
      }
Index: gcc/tree-parloops.c
===================================================================
*** gcc/tree-parloops.c	(revision 143152)
--- gcc/tree-parloops.c	(working copy)
*************** parallelize_loops (void)
*** 1866,1871 ****
--- 1866,1872 ----
  
        changed = true;
        gen_parallel_loop (loop, reduction_list, n_threads, &niter_desc);
+       execute_update_addresses_taken ();
        verify_flow_info ();
        verify_dominators (CDI_DOMINATORS);
        verify_loop_structure ();


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