[PATCH][RFC] PR78035

Jan Hubicka hubicka@ucw.cz
Thu Oct 20 09:46:00 GMT 2016


> 
> The following tries to address the inconsistency with folding
> two addresses based on two decls vs. one pointer vs a decl address
> caused by the latter not honoring interposition.
> 
> Honza, is
> 
> +      /* If obj1 is interposable give up.  */
> +      if (VAR_P (obj1)
> +         && (TREE_STATIC (obj1) || DECL_EXTERNAL (obj1)))
> +       {
> +         varpool_node *node = varpool_node::get (obj1);
> +         if (! node
> +             || ! node->analyzed
> +             || ! decl_binds_to_current_def_p (obj1))
> +           return false;
> +       }
> 
> conservatively correct?  I tried to learn from

I would drop node->analyzed. If decl_binds_to_current_def_p returns
false positives, I would say it is bug of this predicate and also in
late compilation we drop the bodies after compiling but they still
bind to current def.

> symtab_node::equal_address_to but I'm not 100% sure.  Esp.
> in what cases can I expect a NULL node from varpool_node::get?

If you do folding from fold-const.c, then you need to be conservative
when symbol table is not built yet.  If you analyze gimple bodies
after cgraph construction we should be able to assume that the symbol
table entries are created (I know we had some side cases with this in
past but I hope they are fixed or I will fix it).

symtab_node::equal_address_to contains the magic mostly becuase of early
folding.

> 
> Basically I am using the above to decide whether I can use
> the ultimate alias target of two decls to decide whether they
> are not equal (points-to uses the DECL_UID of the ultimate alias
> target in the bitmaps).
> 
> Maybe you can factor out a symtab helper for me?  I think I
> want to catch all cases where symtab_node::equal_address_to
> would return -1 (thus with respect to aliases the above doesn't
> look conservative?)

OK, so you need a predicate such that if it returns true on
node1, node2 then node1==node2 if and only if ultimate alias targets are the
same?

I can do that.  Wonder what would be proper name for this beast?

Note that we are generally inconsistent about what aliases are valid. In older
GCCs I think generally you can't safely use two aliases of the same symbol for
non-readonly memory.

I was thining that we may just impose an requirement for the aliases to be explicitely
declared.  I.e.

extern int a;
extern int b __attribute__ ((alias("a")));

would be only valid way to mix a and b pointing to the same memory location.

The fact that difference declarations are actually different objects is quite
burried into nature of C/C++ standards and trying to fully support this may be
quite some work.  What are the consequences on the base+offset alias oracle
accuracy?

Honza
> 
> Thanks,
> Richard.
> 
> 2016-10-19  Richard Biener  <rguenther@suse.de>
> 
> 	PR tree-optimization/78035
> 	* tree-ssa-alias.h (struct pt_solution): Add vars_contains_interposable
> 	flag.
> 	* tree-ssa-structalias.c: Include varasm.h.
> 	(set_uids_in_ptset): Set vars_contains_interposable for
> 	interposable variables.
> 	(ipa_escaped_pt): Adjust.
> 	* tree-ssa-alias.c: Include varasm.h.
> 	(ptrs_compare_unequal): If pt->vars_contains_interposable or
> 	a decl is interposable then do not conclude anything about
> 	address equality.
> 	(dump_points_to_solution): Handle vars_contains_interposable.
> 	* gimple-pretty-print.c (pp_points_to_solution): Likewise.
> 
> 	* gcc.target/i386/pr78035.c: New testcase.
> 
> 
> Index: gcc/testsuite/gcc.target/i386/pr78035.c
> ===================================================================
> --- gcc/testsuite/gcc.target/i386/pr78035.c	(revision 0)
> +++ gcc/testsuite/gcc.target/i386/pr78035.c	(revision 0)
> @@ -0,0 +1,24 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +
> +extern int a;
> +extern int b;
> +extern int c;
> +
> +int foo(int choose_a)
> +{
> +  int *p;
> +  if (choose_a)
> +    p = &a;
> +  else
> +    p = &b;
> +  return p != &c;
> +}
> +
> +int bar ()
> +{
> +  return &a != &c;
> +}
> +
> +/* We should not optimize away either comparison.  */
> +/* { dg-final { scan-assembler-times "cmp" 2 } } */
> Index: gcc/tree-ssa-alias.c
> ===================================================================
> --- gcc/tree-ssa-alias.c	(revision 241362)
> +++ gcc/tree-ssa-alias.c	(working copy)
> @@ -32,12 +32,12 @@ along with GCC; see the file COPYING3.
>  #include "tree-pretty-print.h"
>  #include "alias.h"
>  #include "fold-const.h"
> -
>  #include "langhooks.h"
>  #include "dumpfile.h"
>  #include "tree-eh.h"
>  #include "tree-dfa.h"
>  #include "ipa-reference.h"
> +#include "varasm.h"
>  
>  /* Broad overview of how alias analysis on gimple works:
>  
> @@ -366,15 +366,39 @@ ptrs_compare_unequal (tree ptr1, tree pt
>        /* We may not use restrict to optimize pointer comparisons.
>           See PR71062.  So we have to assume that restrict-pointed-to
>  	 may be in fact obj1.  */
> -      if (!pi || pi->pt.vars_contains_restrict)
> +      if (! pi
> +	  || pi->pt.vars_contains_restrict
> +	  || pi->pt.vars_contains_interposable)
>  	return false;
> +      /* If obj1 is interposable give up.  */
> +      if (VAR_P (obj1)
> +	  && (TREE_STATIC (obj1) || DECL_EXTERNAL (obj1)))
> +	{
> +	  varpool_node *node = varpool_node::get (obj1);
> +	  if (! node
> +	      || ! node->analyzed
> +	      || ! decl_binds_to_current_def_p (obj1))
> +	    return false;
> +	}
>        return !pt_solution_includes (&pi->pt, obj1);
>      }
>    else if (TREE_CODE (ptr1) == SSA_NAME && obj2)
>      {
>        struct ptr_info_def *pi = SSA_NAME_PTR_INFO (ptr1);
> -      if (!pi || pi->pt.vars_contains_restrict)
> +      if (! pi
> +	  || pi->pt.vars_contains_restrict
> +	  || pi->pt.vars_contains_interposable)
>  	return false;
> +      /* If obj2 is interposable give up.  */
> +      if (VAR_P (obj2)
> +	  && (TREE_STATIC (obj2) || DECL_EXTERNAL (obj2)))
> +	{
> +	  varpool_node *node = varpool_node::get (obj2);
> +	  if (! node
> +	      || ! node->analyzed
> +	      || ! decl_binds_to_current_def_p (obj2))
> +	    return false;
> +	}
>        return !pt_solution_includes (&pi->pt, obj2);
>      }
>  
> @@ -545,7 +569,12 @@ dump_points_to_solution (FILE *file, str
>  	      comma = ", ";
>  	    }
>  	  if (pt->vars_contains_restrict)
> -	    fprintf (file, "%srestrict", comma);
> +	    {
> +	      fprintf (file, "%srestrict", comma);
> +	      comma = ", ";
> +	    }
> +	  if (pt->vars_contains_interposable)
> +	    fprintf (file, "%sinterposable", comma);
>  	  fprintf (file, ")");
>  	}
>      }
> Index: gcc/tree-ssa-alias.h
> ===================================================================
> --- gcc/tree-ssa-alias.h	(revision 241362)
> +++ gcc/tree-ssa-alias.h	(working copy)
> @@ -57,6 +57,8 @@ struct GTY(()) pt_solution
>    /* Nonzero if the vars bitmap includes a anonymous variable used to
>       represent storage pointed to by a restrict qualified pointer.  */
>    unsigned int vars_contains_restrict : 1;
> +  /* Nonzero if the vars bitmap includes an interposable variable.  */
> +  unsigned int vars_contains_interposable : 1;
>  
>    /* Set of variables that this pointer may point to.  */
>    bitmap vars;
> Index: gcc/gimple-pretty-print.c
> ===================================================================
> --- gcc/gimple-pretty-print.c	(revision 241362)
> +++ gcc/gimple-pretty-print.c	(working copy)
> @@ -728,6 +728,12 @@ pp_points_to_solution (pretty_printer *b
>  	    {
>  	      pp_string (buffer, comma);
>  	      pp_string (buffer, "restrict");
> +	      comma = ", ";
> +	    }
> +	  if (pt->vars_contains_interposable)
> +	    {
> +	      pp_string (buffer, comma);
> +	      pp_string (buffer, "interposable");
>  	    }
>  	  pp_string (buffer, ")");
>  	}
> Index: gcc/tree-ssa-structalias.c
> ===================================================================
> --- gcc/tree-ssa-structalias.c	(revision 241362)
> +++ gcc/tree-ssa-structalias.c	(working copy)
> @@ -39,6 +39,8 @@
>  #include "tree-dfa.h"
>  #include "params.h"
>  #include "gimple-walk.h"
> +#include "varasm.h"
> +
>  
>  /* The idea behind this analyzer is to generate set constraints from the
>     program, then solve the resulting constraints in order to generate the
> @@ -6344,6 +6346,18 @@ set_uids_in_ptset (bitmap into, bitmap f
>  		  && fndecl
>  		  && ! auto_var_in_fn_p (vi->decl, fndecl)))
>  	    pt->vars_contains_nonlocal = true;
> +
> +	  /* If we have a variable that is interposable record that fact
> +	     for pointer comparison simplification.  */
> +	  if (VAR_P (vi->decl)
> +	      && (DECL_EXTERNAL (vi->decl) || TREE_PUBLIC (vi->decl)))
> +	    {
> +	      varpool_node *node = varpool_node::get (vi->decl);
> +	      if (! node
> +		  || ! node->analyzed
> +		  || ! decl_binds_to_current_def_p (node->decl))
> +		pt->vars_contains_interposable = true;
> +	    }
>  	}
>  
>        else if (TREE_CODE (vi->decl) == FUNCTION_DECL
> @@ -7576,7 +7590,8 @@ make_pass_build_ealias (gcc::context *ct
>  
>  /* IPA PTA solutions for ESCAPED.  */
>  struct pt_solution ipa_escaped_pt
> -  = { true, false, false, false, false, false, false, false, false, NULL };
> +  = { true, false, false, false, false,
> +      false, false, false, false, false, NULL };
>  
>  /* Associate node with varinfo DATA. Worker for
>     cgraph_for_symbol_thunks_and_aliases.  */



More information about the Gcc-patches mailing list