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] Fix IPA PTA vs. PTA regressions


The following patch takes a stab at fixing the most obvious regression
of IPA PTA vs. PTA - dumbing down of pt_solution_includes_global
with

-   /* ???  This predicate is not correct for the IPA-PTA solution
-      as we do not properly distinguish between unit escape points
-      and global variables.  */
-   if (cfun->gimple_df->ipa_pta)
-     return true;

by making the semantics of pt_solution->vars_contains_nonlocal
(or what composes the NONLOCAL solution) not depend on
the PTA mode (IPA or not).

Optimization-wise this fixes missing DSE and overly conservative
IPA pure-const discovery (only in its less important late invocation).

Note that using IPA PTA still causes optimization regressions because
it does not handle restrict nor the heapvar escape trick and its
escape logic is still overly conservative (probably the biggest issue).

Anyway - the following patch passed bootstrap with -fipa-pta and
regtest on x86_64-unknown-linux-gnu.

Richard.

2015-12-10  Richard Biener  <rguenther@suse.de>

	PR ipa/68331
	* tree-ssa-structalias.c (set_uids_in_ptset): Add fndecl
	parameter and make vars_contains_nonlocal properly have
	function-scope semantics in IPA mode.
	(find_what_var_points_to): Add fndecl parameter.
	(find_what_p_points_to): Likewise.
	(pt_solution_includes_global): Remove IPA PTA early out.
	(compute_points_to_sets): Adjust.
	(ipa_pta_execute): Likewise.  Clear final_solutions after
	each function.

	* gcc.dg/torture/ipa-pta-3.c: New testcase.
	* g++.dg/ipa/ipa-pta-1.C: Likewise.

Index: gcc/tree-ssa-structalias.c
===================================================================
*** gcc/tree-ssa-structalias.c	(revision 231357)
--- gcc/tree-ssa-structalias.c	(working copy)
***************
*** 159,167 ****
  
     The is_global_var bit which marks escape points is overly conservative
     in IPA mode.  Split it to is_escape_point and is_global_var - only
!    externally visible globals are escape points in IPA mode.  This is
!    also needed to fix the pt_solution_includes_global predicate
!    (and thus ptr_deref_may_alias_global_p).
  
     The way we introduce DECL_PT_UID to avoid fixing up all points-to
     sets in the translation unit when we copy a DECL during inlining
--- 159,165 ----
  
     The is_global_var bit which marks escape points is overly conservative
     in IPA mode.  Split it to is_escape_point and is_global_var - only
!    externally visible globals are escape points in IPA mode.
  
     The way we introduce DECL_PT_UID to avoid fixing up all points-to
     sets in the translation unit when we copy a DECL during inlining
***************
*** 186,191 ****
--- 184,190 ----
     propagating it simply like the clobber / uses solutions.  The
     solution can go alongside the non-IPA espaced solution and be
     used to query which vars escape the unit through a function.
+    This is also required to make the escaped-HEAP trick work in IPA mode.
  
     We never put function decls in points-to sets so we do not
     keep the set of called functions for indirect calls.
*************** shared_bitmap_add (bitmap pt_vars)
*** 6101,6107 ****
  /* Set bits in INTO corresponding to the variable uids in solution set FROM.  */
  
  static void
! set_uids_in_ptset (bitmap into, bitmap from, struct pt_solution *pt)
  {
    unsigned int i;
    bitmap_iterator bi;
--- 6100,6107 ----
  /* Set bits in INTO corresponding to the variable uids in solution set FROM.  */
  
  static void
! set_uids_in_ptset (bitmap into, bitmap from, struct pt_solution *pt,
! 		   tree fndecl)
  {
    unsigned int i;
    bitmap_iterator bi;
*************** set_uids_in_ptset (bitmap into, bitmap f
*** 6139,6145 ****
  	  /* Add the decl to the points-to set.  Note that the points-to
  	     set contains global variables.  */
  	  bitmap_set_bit (into, DECL_PT_UID (vi->decl));
! 	  if (vi->is_global_var)
  	    pt->vars_contains_nonlocal = true;
  	}
      }
--- 6139,6157 ----
  	  /* Add the decl to the points-to set.  Note that the points-to
  	     set contains global variables.  */
  	  bitmap_set_bit (into, DECL_PT_UID (vi->decl));
! 	  if (vi->is_global_var
! 	      /* In IPA mode the escaped_heap trick doesn't work as
! 		 ESCAPED is escaped from the unit but
! 		 pt_solution_includes_global needs to answer true for
! 		 all variables not automatic within a function.
! 		 For the same reason is_global_var is not the
! 		 correct flag to track - local variables from other
! 		 functions also need to be considered global.
! 		 Conveniently all HEAP vars are not put in function
! 		 scope.  */
! 	      || (in_ipa_mode
! 		  && fndecl
! 		  && ! auto_var_in_fn_p (vi->decl, fndecl)))
  	    pt->vars_contains_nonlocal = true;
  	}
      }
*************** set_uids_in_ptset (bitmap into, bitmap f
*** 6149,6155 ****
  /* Compute the points-to solution *PT for the variable VI.  */
  
  static struct pt_solution
! find_what_var_points_to (varinfo_t orig_vi)
  {
    unsigned int i;
    bitmap_iterator bi;
--- 6161,6167 ----
  /* Compute the points-to solution *PT for the variable VI.  */
  
  static struct pt_solution
! find_what_var_points_to (tree fndecl, varinfo_t orig_vi)
  {
    unsigned int i;
    bitmap_iterator bi;
*************** find_what_var_points_to (varinfo_t orig_
*** 6214,6220 ****
    finished_solution = BITMAP_GGC_ALLOC ();
    stats.points_to_sets_created++;
  
!   set_uids_in_ptset (finished_solution, vi->solution, pt);
    result = shared_bitmap_lookup (finished_solution);
    if (!result)
      {
--- 6226,6232 ----
    finished_solution = BITMAP_GGC_ALLOC ();
    stats.points_to_sets_created++;
  
!   set_uids_in_ptset (finished_solution, vi->solution, pt, fndecl);
    result = shared_bitmap_lookup (finished_solution);
    if (!result)
      {
*************** find_what_var_points_to (varinfo_t orig_
*** 6233,6239 ****
  /* Given a pointer variable P, fill in its points-to set.  */
  
  static void
! find_what_p_points_to (tree p)
  {
    struct ptr_info_def *pi;
    tree lookup_p = p;
--- 6245,6251 ----
  /* Given a pointer variable P, fill in its points-to set.  */
  
  static void
! find_what_p_points_to (tree fndecl, tree p)
  {
    struct ptr_info_def *pi;
    tree lookup_p = p;
*************** find_what_p_points_to (tree p)
*** 6252,6258 ****
      return;
  
    pi = get_ptr_info (p);
!   pi->pt = find_what_var_points_to (vi);
  }
  
  
--- 6264,6270 ----
      return;
  
    pi = get_ptr_info (p);
!   pi->pt = find_what_var_points_to (fndecl, vi);
  }
  
  
*************** pt_solution_includes_global (struct pt_s
*** 6418,6429 ****
    if (pt->ipa_escaped)
      return pt_solution_includes_global (&ipa_escaped_pt);
  
-   /* ???  This predicate is not correct for the IPA-PTA solution
-      as we do not properly distinguish between unit escape points
-      and global variables.  */
-   if (cfun->gimple_df->ipa_pta)
-     return true;
- 
    return false;
  }
  
--- 6430,6435 ----
*************** compute_points_to_sets (void)
*** 6920,6926 ****
    solve_constraints ();
  
    /* Compute the points-to set for ESCAPED used for call-clobber analysis.  */
!   cfun->gimple_df->escaped = find_what_var_points_to (get_varinfo (escaped_id));
  
    /* Make sure the ESCAPED solution (which is used as placeholder in
       other solutions) does not reference itself.  This simplifies
--- 6926,6933 ----
    solve_constraints ();
  
    /* Compute the points-to set for ESCAPED used for call-clobber analysis.  */
!   cfun->gimple_df->escaped = find_what_var_points_to (cfun->decl,
! 						      get_varinfo (escaped_id));
  
    /* Make sure the ESCAPED solution (which is used as placeholder in
       other solutions) does not reference itself.  This simplifies
*************** compute_points_to_sets (void)
*** 6933,6939 ****
        tree ptr = ssa_name (i);
        if (ptr
  	  && POINTER_TYPE_P (TREE_TYPE (ptr)))
! 	find_what_p_points_to (ptr);
      }
  
    /* Compute the call-used/clobbered sets.  */
--- 6940,6946 ----
        tree ptr = ssa_name (i);
        if (ptr
  	  && POINTER_TYPE_P (TREE_TYPE (ptr)))
! 	find_what_p_points_to (cfun->decl, ptr);
      }
  
    /* Compute the call-used/clobbered sets.  */
*************** compute_points_to_sets (void)
*** 6955,6961 ****
  	    memset (pt, 0, sizeof (struct pt_solution));
  	  else if ((vi = lookup_call_use_vi (stmt)) != NULL)
  	    {
! 	      *pt = find_what_var_points_to (vi);
  	      /* Escaped (and thus nonlocal) variables are always
  	         implicitly used by calls.  */
  	      /* ???  ESCAPED can be empty even though NONLOCAL
--- 6962,6968 ----
  	    memset (pt, 0, sizeof (struct pt_solution));
  	  else if ((vi = lookup_call_use_vi (stmt)) != NULL)
  	    {
! 	      *pt = find_what_var_points_to (cfun->decl, vi);
  	      /* Escaped (and thus nonlocal) variables are always
  	         implicitly used by calls.  */
  	      /* ???  ESCAPED can be empty even though NONLOCAL
*************** compute_points_to_sets (void)
*** 6976,6982 ****
  	    memset (pt, 0, sizeof (struct pt_solution));
  	  else if ((vi = lookup_call_clobber_vi (stmt)) != NULL)
  	    {
! 	      *pt = find_what_var_points_to (vi);
  	      /* Escaped (and thus nonlocal) variables are always
  	         implicitly clobbered by calls.  */
  	      /* ???  ESCAPED can be empty even though NONLOCAL
--- 6983,6989 ----
  	    memset (pt, 0, sizeof (struct pt_solution));
  	  else if ((vi = lookup_call_clobber_vi (stmt)) != NULL)
  	    {
! 	      *pt = find_what_var_points_to (cfun->decl, vi);
  	      /* Escaped (and thus nonlocal) variables are always
  	         implicitly clobbered by calls.  */
  	      /* ???  ESCAPED can be empty even though NONLOCAL
*************** ipa_pta_execute (void)
*** 7516,7522 ****
       ???  Note that the computed escape set is not correct
       for the whole unit as we fail to consider graph edges to
       externally visible functions.  */
!   ipa_escaped_pt = find_what_var_points_to (get_varinfo (escaped_id));
  
    /* Make sure the ESCAPED solution (which is used as placeholder in
       other solutions) does not reference itself.  This simplifies
--- 7523,7529 ----
       ???  Note that the computed escape set is not correct
       for the whole unit as we fail to consider graph edges to
       externally visible functions.  */
!   ipa_escaped_pt = find_what_var_points_to (NULL, get_varinfo (escaped_id));
  
    /* Make sure the ESCAPED solution (which is used as placeholder in
       other solutions) does not reference itself.  This simplifies
*************** ipa_pta_execute (void)
*** 7542,7548 ****
  	{
  	  if (ptr
  	      && POINTER_TYPE_P (TREE_TYPE (ptr)))
! 	    find_what_p_points_to (ptr);
  	}
  
        /* Compute the call-use and call-clobber sets for indirect calls
--- 7549,7555 ----
  	{
  	  if (ptr
  	      && POINTER_TYPE_P (TREE_TYPE (ptr)))
! 	    find_what_p_points_to (node->decl, ptr);
  	}
  
        /* Compute the call-use and call-clobber sets for indirect calls
*************** ipa_pta_execute (void)
*** 7576,7585 ****
  		{
  		  *gimple_call_clobber_set (stmt)
  		     = find_what_var_points_to
! 		         (first_vi_for_offset (fi, fi_clobbers));
  		  *gimple_call_use_set (stmt)
  		     = find_what_var_points_to
! 		         (first_vi_for_offset (fi, fi_uses));
  		}
  	      /* Handle direct calls to external functions.  */
  	      else if (decl)
--- 7583,7592 ----
  		{
  		  *gimple_call_clobber_set (stmt)
  		     = find_what_var_points_to
! 		         (node->decl, first_vi_for_offset (fi, fi_clobbers));
  		  *gimple_call_use_set (stmt)
  		     = find_what_var_points_to
! 		         (node->decl, first_vi_for_offset (fi, fi_uses));
  		}
  	      /* Handle direct calls to external functions.  */
  	      else if (decl)
*************** ipa_pta_execute (void)
*** 7589,7595 ****
  		    memset (pt, 0, sizeof (struct pt_solution));
  		  else if ((vi = lookup_call_use_vi (stmt)) != NULL)
  		    {
! 		      *pt = find_what_var_points_to (vi);
  		      /* Escaped (and thus nonlocal) variables are always
  			 implicitly used by calls.  */
  		      /* ???  ESCAPED can be empty even though NONLOCAL
--- 7596,7602 ----
  		    memset (pt, 0, sizeof (struct pt_solution));
  		  else if ((vi = lookup_call_use_vi (stmt)) != NULL)
  		    {
! 		      *pt = find_what_var_points_to (node->decl, vi);
  		      /* Escaped (and thus nonlocal) variables are always
  			 implicitly used by calls.  */
  		      /* ???  ESCAPED can be empty even though NONLOCAL
*************** ipa_pta_execute (void)
*** 7610,7616 ****
  		    memset (pt, 0, sizeof (struct pt_solution));
  		  else if ((vi = lookup_call_clobber_vi (stmt)) != NULL)
  		    {
! 		      *pt = find_what_var_points_to (vi);
  		      /* Escaped (and thus nonlocal) variables are always
  			 implicitly clobbered by calls.  */
  		      /* ???  ESCAPED can be empty even though NONLOCAL
--- 7617,7623 ----
  		    memset (pt, 0, sizeof (struct pt_solution));
  		  else if ((vi = lookup_call_clobber_vi (stmt)) != NULL)
  		    {
! 		      *pt = find_what_var_points_to (node->decl, vi);
  		      /* Escaped (and thus nonlocal) variables are always
  			 implicitly clobbered by calls.  */
  		      /* ???  ESCAPED can be empty even though NONLOCAL
*************** ipa_pta_execute (void)
*** 7670,7682 ****
  			  if (!uses->anything)
  			    {
  			      sol = find_what_var_points_to
! 				      (first_vi_for_offset (vi, fi_uses));
  			      pt_solution_ior_into (uses, &sol);
  			    }
  			  if (!clobbers->anything)
  			    {
  			      sol = find_what_var_points_to
! 				      (first_vi_for_offset (vi, fi_clobbers));
  			      pt_solution_ior_into (clobbers, &sol);
  			    }
  			}
--- 7677,7691 ----
  			  if (!uses->anything)
  			    {
  			      sol = find_what_var_points_to
! 				      (node->decl,
! 				       first_vi_for_offset (vi, fi_uses));
  			      pt_solution_ior_into (uses, &sol);
  			    }
  			  if (!clobbers->anything)
  			    {
  			      sol = find_what_var_points_to
! 				      (node->decl,
! 				       first_vi_for_offset (vi, fi_clobbers));
  			      pt_solution_ior_into (clobbers, &sol);
  			    }
  			}
*************** ipa_pta_execute (void)
*** 7686,7691 ****
--- 7695,7706 ----
  	}
  
        fn->gimple_df->ipa_pta = true;
+ 
+       /* We have to re-set the final-solution cache after each function
+          because what is a "global" is dependent on function context.  */
+       final_solutions->empty ();
+       obstack_free (&final_solutions_obstack, NULL);
+       gcc_obstack_init (&final_solutions_obstack);
      }
  
    delete_points_to_sets ();
Index: gcc/testsuite/gcc.dg/torture/ipa-pta-3.c
===================================================================
*** gcc/testsuite/gcc.dg/torture/ipa-pta-3.c	(revision 0)
--- gcc/testsuite/gcc.dg/torture/ipa-pta-3.c	(working copy)
***************
*** 0 ****
--- 1,19 ----
+ /* { dg-do run } */
+ /* { dg-additional-options "-fipa-pta" } */
+ 
+ extern void abort (void);
+ extern void *malloc (__SIZE_TYPE__);
+ 
+ static int *p;
+ static void __attribute__((noinline,noclone)) foo ()
+ {
+   p = (int *) malloc (24);
+   *p = 2;
+ }
+ int main()
+ {
+   foo ();
+   if (*p != 2)
+     abort ();
+   return 0;
+ }
Index: gcc/testsuite/g++.dg/ipa/ipa-pta-1.C
===================================================================
--- gcc/testsuite/g++.dg/ipa/ipa-pta-1.C	(revision 0)
+++ gcc/testsuite/g++.dg/ipa/ipa-pta-1.C	(working copy)
@@ -0,0 +1,35 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -fipa-pta"  } */
+
+struct A {                                                                      
+  A() {ptr=&b;}                                                                 
+  A(const A &a) {ptr = &b;}                                                     
+  void test() { if (ptr != &b) __builtin_abort ();}                             
+  int b;                                                                        
+  int *ptr;                                                                     
+};                                                                              
+
+A test1(A a)
+{
+  a.test();
+  return a;
+}
+A test2(A a)
+{
+  a.test();
+  return a;
+}
+__attribute__ ((noinline))
+static void
+test_me (A (*t)(A))
+{
+  struct A a, b=t(a);
+  b.test ();
+}
+int
+main()
+{
+  test_me (test1);
+  test_me (test2);
+  return 0;
+}


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