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]

Re: [PATCH] Fix PR36343, TBAA-pruning and call-clobbering broken


On Tue, 27 May 2008, Richard Guenther wrote:

> 
> The minor amount of cases we ever do TBAA-pruning of points-to-sets break
> the call-clobbering code which works on these (pruned) points-to-sets.
> 
> TBAA-pruning only ever happens for points-to-sets of pointers that
> point to a pointer (and are dereferenced).  While this probably wasn't
> intended it saved us from more widespread breakage due to this bug.
> 
> Fortunately this enables the following simple fix suitable for the 4.3
> branch which simply rips out the TBAA-pruning of points-to-sets.  The
> proper fix for the mainline is in the works (but I still intend to do
> this intermediate step), which is to completely rework the call-clobbering
> code to work on the points-to solution instead.  I'll file a few PRs
> regarding the missed optimizations and the latent bugs.
> 
> Bootstrap and regtest running.  I'll apply this after it finished.

It went fine, but for trunk I'm evaluating the following which implements
the do-call-clobbering with the unpruned sets instead.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

2008-05-27  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/36343
	PR tree-optimization/36346
	* tree-flow.h (clobber_what_p_points_to): Declare.
	* tree-ssa-structalias.c (set_uids_in_ptset): Whether the
	pointed-to variable is dereferenced is irrelevant to whether
	the pointer can access the pointed-to variable.
	(clobber_what_p_points_to): New function.
	* tree-ssa-alias.c (set_initial_properties): Use it.

	* gcc.c-torture/execute/pr36343.c: New testcase.

Index: gcc/tree-flow.h
===================================================================
*** gcc/tree-flow.h	(revision 136029)
--- gcc/tree-flow.h	(working copy)
*************** tree gimple_fold_indirect_ref (tree);
*** 1171,1176 ****
--- 1171,1177 ----
  
  /* In tree-ssa-structalias.c */
  bool find_what_p_points_to (tree);
+ bool clobber_what_p_points_to (tree);
  
  /* In tree-ssa-live.c */
  extern void remove_unused_locals (void);
Index: gcc/tree-ssa-alias.c
===================================================================
*** gcc/tree-ssa-alias.c	(revision 136030)
--- gcc/tree-ssa-alias.c	(working copy)
*************** set_initial_properties (struct alias_inf
*** 554,577 ****
  	  if (tag)
  	    mark_call_clobbered (tag, pi->escape_mask);
  
! 	  if (pi->pt_vars)
! 	    {
! 	      bitmap_iterator bi;
! 	      unsigned int j;	      
! 	      EXECUTE_IF_SET_IN_BITMAP (pi->pt_vars, 0, j, bi)
! 		{
! 		  tree alias = referenced_var (j);
! 
! 		  /* If you clobber one part of a structure, you
! 		     clobber the entire thing.  While this does not make
! 		     the world a particularly nice place, it is necessary
! 		     in order to allow C/C++ tricks that involve
! 		     pointer arithmetic to work.  */
! 		  if (!unmodifiable_var_p (alias))
! 		    mark_call_clobbered (alias, pi->escape_mask);
! 		}
! 	    }
! 	  else if (pi->pt_anything)
  	    {
  	      bitmap_iterator bi;
  	      unsigned int j;
--- 554,562 ----
  	  if (tag)
  	    mark_call_clobbered (tag, pi->escape_mask);
  
! 	  /* Defer to points-to analysis if possible, otherwise
! 	     clobber all addressable variables.  */
! 	  if (!clobber_what_p_points_to (ptr))
  	    {
  	      bitmap_iterator bi;
  	      unsigned int j;
Index: gcc/testsuite/gcc.c-torture/execute/pr36343.c
===================================================================
*** gcc/testsuite/gcc.c-torture/execute/pr36343.c	(revision 0)
--- gcc/testsuite/gcc.c-torture/execute/pr36343.c	(revision 0)
***************
*** 0 ****
--- 1,32 ----
+ extern void abort (void);
+ 
+ void __attribute__((noinline))
+ bar (int **p)
+ {
+   float *q = (float *)p;
+   *q = 0.0;
+ }
+ 
+ float __attribute__((noinline))
+ foo (int b)
+ {
+   int *i = 0;
+   float f = 1.0;
+   int **p;
+   if (b)
+     p = &i;
+   else
+     p = (int **)&f;
+   bar (p);
+   if (b)
+     return **p;
+   return f;
+ }
+ 
+ int main()
+ {
+   if (foo(0) != 0.0)
+     abort ();
+   return 0;
+ }
+ 
Index: gcc/tree-ssa-structalias.c
===================================================================
*** gcc/tree-ssa-structalias.c	(revision 136029)
--- gcc/tree-ssa-structalias.c	(working copy)
*************** set_uids_in_ptset (tree ptr, bitmap into
*** 4664,4680 ****
  	  || TREE_CODE (vi->decl) == RESULT_DECL)
  	{
  	  /* Just add VI->DECL to the alias set.
! 	     Don't type prune artificial vars.  */
! 	  if (vi->is_artificial_var)
  	    bitmap_set_bit (into, DECL_UID (vi->decl));
  	  else
  	    {
  	      alias_set_type var_alias_set, ptr_alias_set;
  	      var_alias_set = get_alias_set (vi->decl);
  	      ptr_alias_set = get_alias_set (TREE_TYPE (TREE_TYPE (ptr)));
! 	      if (no_tbaa_pruning
! 		  || (!is_derefed && !vi->directly_dereferenced)
! 		  || alias_sets_conflict_p (ptr_alias_set, var_alias_set))
  	        bitmap_set_bit (into, DECL_UID (vi->decl));
  	    }
  	}
--- 4664,4682 ----
  	  || TREE_CODE (vi->decl) == RESULT_DECL)
  	{
  	  /* Just add VI->DECL to the alias set.
! 	     Don't type prune artificial vars or points-to sets
! 	     for pointers that have not been dereferenced or with
! 	     type-based pruning disabled.  */
! 	  if (vi->is_artificial_var
! 	      || !is_derefed
! 	      || no_tbaa_pruning)
  	    bitmap_set_bit (into, DECL_UID (vi->decl));
  	  else
  	    {
  	      alias_set_type var_alias_set, ptr_alias_set;
  	      var_alias_set = get_alias_set (vi->decl);
  	      ptr_alias_set = get_alias_set (TREE_TYPE (TREE_TYPE (ptr)));
! 	      if (alias_sets_conflict_p (ptr_alias_set, var_alias_set))
  	        bitmap_set_bit (into, DECL_UID (vi->decl));
  	    }
  	}
*************** find_what_p_points_to (tree p)
*** 4885,4891 ****
--- 4887,4969 ----
    return false;
  }
  
+ /* Mark everything that p points to as call clobbered.  Returns true
+    if everything is done and false if all addressable variables need to
+    be clobbered because p points to anything.  */
  
+ bool
+ clobber_what_p_points_to (tree p)
+ {
+   tree lookup_p = p;
+   varinfo_t vi;
+ 
+   if (!have_alias_info)
+     return false;
+ 
+   /* For parameters, get at the points-to set for the actual parm
+      decl.  */
+   if (TREE_CODE (p) == SSA_NAME
+       && TREE_CODE (SSA_NAME_VAR (p)) == PARM_DECL
+       && SSA_NAME_IS_DEFAULT_DEF (p))
+     lookup_p = SSA_NAME_VAR (p);
+ 
+   vi = lookup_vi_for_tree (lookup_p);
+   if (vi)
+     {
+       if (vi->is_artificial_var)
+ 	return false;
+ 
+       /* See if this is a field or a structure.  */
+       if (vi->size != vi->fullsize)
+ 	{
+ 	  /* Nothing currently asks about structure fields directly,
+ 	     but when they do, we need code here to hand back the
+ 	     points-to set.  */
+ 	  return false;
+ 	}
+       else
+ 	{
+ 	  struct ptr_info_def *pi = get_ptr_info (p);
+ 	  unsigned int i;
+ 	  bitmap_iterator bi;
+ 
+ 	  /* This variable may have been collapsed, let's get the real
+ 	     variable.  */
+ 	  vi = get_varinfo (find (vi->id));
+ 
+ 	  /* Translate artificial variables into SSA_NAME_PTR_INFO
+ 	     attributes.  */
+ 	  EXECUTE_IF_SET_IN_BITMAP (vi->solution, 0, i, bi)
+ 	    {
+ 	      varinfo_t vi = get_varinfo (i);
+ 
+ 	      if (vi->is_artificial_var)
+ 		{
+ 		  /* nothing_id and readonly_id do not cause any
+ 		     call clobber ops.  For anything_id and integer_id
+ 		     we need to clobber all addressable vars.  */
+ 		  if (vi->id == anything_id
+ 		      || vi->id == integer_id)
+ 		    return false;
+ 		}
+ 
+ 	      /* Only artificial heap-vars are further interesting.  */
+ 	      if (vi->is_artificial_var && !vi->is_heap_var)
+ 		continue;
+ 
+ 	      if ((TREE_CODE (vi->decl) == VAR_DECL
+ 		   || TREE_CODE (vi->decl) == PARM_DECL
+ 		   || TREE_CODE (vi->decl) == RESULT_DECL)
+ 		  && !unmodifiable_var_p (vi->decl))
+ 		mark_call_clobbered (vi->decl, pi->escape_mask);
+ 	    }
+ 
+ 	  return true;
+ 	}
+     }
+ 
+   return false;
+ }
  
  /* Dump points-to information to OUTFILE.  */
  


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