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

Richard Guenther rguenther@suse.de
Fri Jan 9 12:29:00 GMT 2009


On Thu, 8 Jan 2009, Richard Guenther wrote:

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

OTOH that is ugly.  Better to fix TREE_ADDRESSABLE handling.  So I
reverted part of this patch.

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to the 
branch.

Richard.

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

	Revert
	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-ssa-loop.c (tree_ssa_loop_ivopts): Update address-taken.
	* 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 (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 143185)
--- gcc/tree-into-ssa.c	(working copy)
*************** struct gimple_opt_pass pass_build_ssa =
*** 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 */
--- 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 */
Index: gcc/tree-ssa-loop.c
===================================================================
*** gcc/tree-ssa-loop.c	(revision 143185)
--- gcc/tree-ssa-loop.c	(working copy)
*************** tree_ssa_loop_ivopts (void)
*** 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
--- 665,671 ----
      return 0;
  
    tree_ssa_iv_optimize ();
!   return 0;
  }
  
  static bool
Index: gcc/tree-parloops.c
===================================================================
*** gcc/tree-parloops.c	(revision 143185)
--- gcc/tree-parloops.c	(working copy)
*************** parallelize_loops (void)
*** 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 ();
--- 1866,1871 ----
Index: gcc/tree-vectorizer.c
===================================================================
*** gcc/tree-vectorizer.c	(revision 143185)
--- gcc/tree-vectorizer.c	(working copy)
*************** vectorize_loops (void)
*** 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;
--- 2818,2823 ----
*************** vectorize_loops (void)
*** 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.
--- 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.
Index: gcc/tree-inline.c
===================================================================
*** gcc/tree-inline.c	(revision 143185)
--- gcc/tree-inline.c	(working copy)
*************** optimize_inline_calls (tree fn)
*** 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));
  }
  
--- 3570,3575 ----
*************** tree_function_versioning (tree old_decl,
*** 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);
--- 4363,4368 ----
Index: gcc/tree-cfg.c
===================================================================
*** gcc/tree-cfg.c	(revision 143185)
--- gcc/tree-cfg.c	(working copy)
*************** gimple_split_edge (edge edge_in)
*** 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))
--- 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))
*************** verify_expr (tree *tp, int *walk_subtree
*** 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;
        }
  
--- 2872,2877 ----
*************** verify_stmt (gimple_stmt_iterator *gsi)
*** 4075,4081 ****
      }
  
    memset (&wi, 0, sizeof (wi));
-   wi.gsi = *gsi;
    addr = walk_gimple_op (gsi_stmt (*gsi), verify_expr, &wi);
    if (addr)
      {
--- 4055,4060 ----
Index: gcc/passes.c
===================================================================
*** gcc/passes.c	(revision 143185)
--- gcc/passes.c	(working copy)
*************** init_optimization_passes (void)
*** 586,591 ****
--- 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);



More information about the Gcc-patches mailing list