On Thu, 20 Oct 2016, Jan Hubicka wrote:
> >
> > 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.
Ok.
> > 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.
I see. As this is part of the code happens when points-to is available
we should be safe then, but I guess "safe" would be to handle ! node
conservatively then.
> >
> > 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?
Yes.
> I can do that. Wonder what would be proper name for this beast?
Is it equal to node1 and node2 being not interposable? (if we
factor out nonnull-ness)
If so interposable ()?
Not sure if this captures the alias behavior.
> 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?
No idea but part of PR78035 notes that we are conservative when
comparing pointers but quite optimistic for aliasing here. This
causes issue when we propagate pointer equivalences, like
extern int a;
extern int b;
ptr = &a;
...
if (ptr == &b)
{
*ptr = 3;
.. = a;
}
where we end up with
b = 3;
.. = a;
and thus breaking aliasing when a and b are the same.
But interposition would mean that (without LTO) all globals are
to be treated the same by points-to and we'd be left with
TBAA for the disambiguations? At least for externs and for
all globals with -fPIC.
I guess the effect would be pretty bad...
(I'm trying to "fix" PR77964)
Richard.
> Honza
> >
> > Thanks,
> > Richard.
> >
> > 2016-10-19 Richard Biener <rguent...@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. */
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
