This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR39207, bogus strict-aliasing warning, PR39074, wrong PTA
- From: Daniel Berlin <dberlin at dberlin dot org>
- To: Richard Guenther <rguenther at suse dot de>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 18 Feb 2009 12:20:45 -0500
- Subject: Re: [PATCH] Fix PR39207, bogus strict-aliasing warning, PR39074, wrong PTA
- References: <alpine.LNX.2.00.0902181444120.11836@zhemvz.fhfr.qr>
On Wed, Feb 18, 2009 at 8:47 AM, Richard Guenther <rguenther@suse.de> wrote:
>
> This "backports" the fix for PR39074 from alias-improvements branch to
> trunk, thereby fixing PR39207, bogus strict-aliasing warnings from
> libstdc++ headers. On the branch I needed the second patch appended
> below to avoid performance regressions, so I will backport this as well.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu. I have put the
> patches on x86_64 and ia64 SPEC2000 testing (I hope we're not going
> the 4.3 way of very late performance regressions due to bug fixes...).
>
> I plan to apply this tomorrow after testresults have arrived.
>
> Richard.
>
> 2009-02-18 Richard Guenther <rguenther@suse.de>
>
> PR tree-optimization/39207
> PR tree-optimization/39074
> * tree-ssa-structalias.c (storedanything_id, var_storedanything,
> storedanything_tree): New.
> (do_ds_constraint): Simplify ANYTHING shortcutting. Update
> the STOREDANYTHING solution if the lhs solution contains
> ANYTHING.
> (build_pred_graph): Add edges from STOREDANYTHING to all
> non-direct nodes.
You mean build_succ_graph.
Also, you have the comment and the description wrong.
It's really adding edges *to* STOREDANYTHING *from* all non-direct nodes.
> (init_base_vars): Initialize STOREDANYTHING.
> (compute_points_to_sets): Free substitution info after
> building the succ graph.
> (ipa_pta_execute): Likewise.
>
> * gcc.dg/torture/pr39074.c: New testcase.
> * gcc.dg/torture/pr39074-2.c: Likewise.
> * gcc.dg/torture/pr39074-3.c: Likewise.
>
> Index: gcc/tree-ssa-structalias.c
> ===================================================================
> *** gcc/tree-ssa-structalias.c (revision 144259)
> --- gcc/tree-ssa-structalias.c (working copy)
> *************** get_varinfo_fc (unsigned int n)
> *** 297,303 ****
>
> /* Static IDs for the special variables. */
> enum { nothing_id = 0, anything_id = 1, readonly_id = 2,
> ! escaped_id = 3, nonlocal_id = 4, callused_id = 5, integer_id = 6 };
>
> /* Variable that represents the unknown pointer. */
> static varinfo_t var_anything;
> --- 297,304 ----
>
> /* Static IDs for the special variables. */
> enum { nothing_id = 0, anything_id = 1, readonly_id = 2,
> ! escaped_id = 3, nonlocal_id = 4, callused_id = 5,
> ! storedanything_id = 6, integer_id = 7 };
>
> /* Variable that represents the unknown pointer. */
> static varinfo_t var_anything;
> *************** static tree nonlocal_tree;
> *** 323,328 ****
> --- 324,333 ----
> static varinfo_t var_callused;
> static tree callused_tree;
>
> + /* Variable that represents variables that are stored to anything. */
> + static varinfo_t var_storedanything;
> + static tree storedanything_tree;
> +
> /* Variable that represents integers. This is used for when people do things
> like &0->a.b. */
> static varinfo_t var_integer;
> *************** build_pred_graph (void)
> *** 1182,1188 ****
> static void
> build_succ_graph (void)
> {
> ! int i;
> constraint_t c;
>
> for (i = 0; VEC_iterate (constraint_t, constraints, i, c); i++)
> --- 1187,1193 ----
> static void
> build_succ_graph (void)
> {
> ! unsigned i, t;
> constraint_t c;
>
> for (i = 0; VEC_iterate (constraint_t, constraints, i, c); i++)
> *************** build_succ_graph (void)
> *** 1223,1228 ****
> --- 1228,1241 ----
> add_graph_edge (graph, lhsvar, rhsvar);
> }
> }
> +
> + /* Add edges from STOREDANYTHING to all non-direct nodes. */
> + t = find (storedanything_id);
> + for (i = integer_id + 1; i < FIRST_REF_NODE; ++i)
> + {
> + if (!TEST_BIT (graph->direct_nodes, i))
> + add_graph_edge (graph, find (i), t);
> + }
> }
>
>
> *************** do_ds_constraint (constraint_t c, bitmap
> *** 1608,1642 ****
> unsigned int j;
> bitmap_iterator bi;
>
> ! if (bitmap_bit_p (sol, anything_id))
> ! {
> ! EXECUTE_IF_SET_IN_BITMAP (delta, 0, j, bi)
> ! {
> ! varinfo_t jvi = get_varinfo (j);
> ! unsigned int t;
> ! unsigned int loff = c->lhs.offset;
> ! unsigned HOST_WIDE_INT fieldoffset = jvi->offset + loff;
> ! varinfo_t v;
> !
> ! v = get_varinfo (j);
> ! if (!v->is_full_var)
> ! {
> ! v = first_vi_for_offset (v, fieldoffset);
> ! /* If the access is outside of the variable we can ignore it. */
> ! if (!v)
> ! continue;
> ! }
> ! t = find (v->id);
> !
> ! if (bitmap_set_bit (get_varinfo (t)->solution, anything_id)
> ! && !TEST_BIT (changed, t))
> ! {
> ! SET_BIT (changed, t);
> ! changed_count++;
> ! }
> ! }
> ! return;
> ! }
>
> /* For each member j of delta (Sol(x)), add an edge from y to j and
> union Sol(y) into Sol(j) */
> --- 1621,1653 ----
> unsigned int j;
> bitmap_iterator bi;
>
> ! /* Our IL does not allow this. */
> ! gcc_assert (c->rhs.offset == 0);
> !
> ! /* If the solution of y contains ANYTHING simply use the ANYTHING
> ! solution. This avoids needlessly increasing the points-to sets. */
> ! if (bitmap_bit_p (sol, anything_id))
> ! sol = get_varinfo (find (anything_id))->solution;
> !
> ! /* If the solution for x contains ANYTHING we have to merge the
> ! solution of y into all pointer variables which we do via
> ! STOREDANYTHING. */
> ! if (bitmap_bit_p (delta, anything_id))
> ! {
> ! unsigned t = find (storedanything_id);
> ! if (add_graph_edge (graph, t, rhs))
> ! {
> ! if (bitmap_ior_into (get_varinfo (t)->solution, sol))
> ! {
> ! if (!TEST_BIT (changed, t))
> ! {
> ! SET_BIT (changed, t);
> ! changed_count++;
> ! }
> ! }
> ! }
> ! return;
> ! }
>
> /* For each member j of delta (Sol(x)), add an edge from y to j and
> union Sol(y) into Sol(j) */
> *************** init_base_vars (void)
> *** 5239,5244 ****
> --- 5250,5268 ----
> rhs.offset = 0;
> process_constraint (new_constraint (lhs, rhs));
>
> + /* Create the STOREDANYTHING variable, used to represent the set of
> + variables stored to *ANYTHING. */
> + storedanything_tree = create_tmp_var_raw (ptr_type_node, "STOREDANYTHING");
> + var_storedanything = new_var_info (storedanything_tree, storedanything_id,
> + "STOREDANYTHING");
> + insert_vi_for_tree (storedanything_tree, var_storedanything);
> + var_storedanything->is_artificial_var = 1;
> + var_storedanything->offset = 0;
> + var_storedanything->size = ~0;
> + var_storedanything->fullsize = ~0;
> + var_storedanything->is_special_var = 0;
> + VEC_safe_push (varinfo_t, heap, varmap, var_storedanything);
> +
> /* Create the INTEGER variable, used to represent that a variable points
> to an INTEGER. */
> integer_tree = create_tmp_var_raw (void_type_node, "INTEGER");
> *************** compute_points_to_sets (void)
> *** 5537,5545 ****
> fprintf (dump_file, "Rewriting constraints and unifying "
> "variables\n");
> rewrite_constraints (graph, si);
> - free_var_substitution_info (si);
>
> build_succ_graph ();
>
> if (dump_file && (dump_flags & TDF_GRAPH))
> dump_constraint_graph (dump_file);
> --- 5561,5569 ----
> fprintf (dump_file, "Rewriting constraints and unifying "
> "variables\n");
> rewrite_constraints (graph, si);
>
> build_succ_graph ();
> + free_var_substitution_info (si);
>
> if (dump_file && (dump_flags & TDF_GRAPH))
> dump_constraint_graph (dump_file);
> *************** ipa_pta_execute (void)
> *** 5698,5706 ****
> build_pred_graph ();
> si = perform_var_substitution (graph);
> rewrite_constraints (graph, si);
> - free_var_substitution_info (si);
>
> build_succ_graph ();
> move_complex_constraints (graph);
> unite_pointer_equivalences (graph);
> find_indirect_cycles (graph);
> --- 5722,5730 ----
> build_pred_graph ();
> si = perform_var_substitution (graph);
> rewrite_constraints (graph, si);
>
> build_succ_graph ();
> + free_var_substitution_info (si);
> move_complex_constraints (graph);
> unite_pointer_equivalences (graph);
> find_indirect_cycles (graph);
> Index: gcc/testsuite/gcc.dg/torture/pr39074-2.c
> ===================================================================
> *** gcc/testsuite/gcc.dg/torture/pr39074-2.c (revision 0)
> --- gcc/testsuite/gcc.dg/torture/pr39074-2.c (revision 0)
> ***************
> *** 0 ****
> --- 1,34 ----
> + /* { dg-do run } */
> + /* { dg-require-effective-target stdint_types } */
> + /* { dg-options "-fdump-tree-alias" } */
> + /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
> +
> + #include <stdint.h>
> +
> + int i;
> + uintptr_t __attribute__((noinline,const)) bar(int ***p) { return (uintptr_t)p; }
> + void __attribute__((noinline))
> + foo(void)
> + {
> + int *y;
> + int **a = &y, **x;
> + int ***p;
> + uintptr_t b;
> + b = bar(&a);
> + p = (int ***)b;
> + x = *p;
> + *x = &i; /* *ANYTHING = &i has to make sure that y points to i. */
> + *y = 0;
> + }
> + extern void abort (void);
> + int main()
> + {
> + i = 1;
> + foo ();
> + if (i != 0)
> + abort ();
> + return 0;
> + }
> +
> + /* { dg-final { scan-tree-dump "y.._., name memory tag: NMT..., is dereferenced, points-to vars: { i }" "alias" } } */
> + /* { dg-final { cleanup-tree-dump "alias" } } */
> Index: gcc/testsuite/gcc.dg/torture/pr39074-3.c
> ===================================================================
> *** gcc/testsuite/gcc.dg/torture/pr39074-3.c (revision 0)
> --- gcc/testsuite/gcc.dg/torture/pr39074-3.c (revision 0)
> ***************
> *** 0 ****
> --- 1,25 ----
> + /* { dg-do run } */
> + /* { dg-require-effective-target stdint_types } */
> +
> + #include <stdint.h>
> +
> + uintptr_t __attribute__((noinline,const)) bar(int ***p) { return (uintptr_t)p; }
> + extern void abort (void);
> + int main()
> + {
> + int i, j;
> + int *y = &j;
> + int **a = &y, **x;
> + int ***p;
> + uintptr_t b;
> + b = bar(&a);
> + p = (int ***)b;
> + x = *p;
> + *x = &i;
> + i = 1;
> + *y = 0;
> + if (i != 0)
> + abort ();
> + return 0;
> + }
> +
> Index: gcc/testsuite/gcc.dg/torture/pr39074.c
> ===================================================================
> *** gcc/testsuite/gcc.dg/torture/pr39074.c (revision 0)
> --- gcc/testsuite/gcc.dg/torture/pr39074.c (revision 0)
> ***************
> *** 0 ****
> --- 1,31 ----
> + /* { dg-do run } */
> + /* { dg-options "-fdump-tree-alias" } */
> + /* { dg-skip-if "" { *-*-* } { "-O0" } { "" } } */
> +
> + int i;
> + void __attribute__((noinline))
> + foo(long b, long q)
> + {
> + int *y;
> + int **a = &y, **x;
> + int ***p;
> + if (b)
> + p = (int ***)q;
> + else
> + p = &a;
> + x = *p;
> + *x = &i; /* *ANYTHING = &i has to make sure that y points to i. */
> + *y = 0;
> + }
> + extern void abort (void);
> + int main()
> + {
> + i = 1;
> + foo (0, 0);
> + if (i != 0)
> + abort ();
> + return 0;
> + }
> +
> + /* { dg-final { scan-tree-dump "y.._., name memory tag: NMT..., is dereferenced, points-to vars: { i }" "alias" } } */
> + /* { dg-final { cleanup-tree-dump "alias" } } */
>
>
> 2009-02-18 Richard Guenther <rguenther@suse.de>
>
> * tree-ssa-structalias.c (struct variable_info): Add may_have_pointers
> field.
> (do_ds_constraint): Do not add to special var or non-pointer
> field solutions.
> (type_could_have_pointers): Split out from ...
> (could_have_pointers): ... here. For arrays use the element type.
> (create_variable_info_for): Initialize may_have_pointers.
> (new_var_info): Likewise.
> (handle_lhs_call): Make the HEAP variable unknown-sized.
> (intra_create_variable_infos): Use a type with pointers for
> PARM_NOALIAS, make it unknown-sized.
>
> Index: gcc/tree-ssa-structalias.c
> ===================================================================
> --- gcc/tree-ssa-structalias.c.orig 2009-02-18 13:32:32.000000000 +0100
> +++ gcc/tree-ssa-structalias.c 2009-02-18 13:32:05.000000000 +0100
> @@ -230,6 +230,9 @@ struct variable_info
> variable. This is used for C++ placement new. */
> unsigned int no_tbaa_pruning : 1;
>
> + /* True if this field may contain pointers. */
> + unsigned int may_have_pointers : 1;
> +
> /* Variable id this was collapsed to due to type unsafety. Zero if
> this variable was not collapsed. This should be unused completely
> after build_succ_graph, or something is broken. */
> @@ -382,6 +385,7 @@ new_var_info (tree t, unsigned int id, c
> ret->is_special_var = false;
> ret->is_unknown_size_var = false;
> ret->is_full_var = false;
> + ret->may_have_pointers = true;
> var = t;
> if (TREE_CODE (var) == SSA_NAME)
> var = SSA_NAME_VAR (var);
> @@ -1659,26 +1663,27 @@ do_ds_constraint (constraint_t c, bitmap
> varinfo_t v;
> unsigned int t;
> unsigned HOST_WIDE_INT fieldoffset = get_varinfo (j)->offset + loff;
> - bitmap tmp;
>
> v = first_vi_for_offset (get_varinfo (j), fieldoffset);
> /* If the access is outside of the variable we can ignore it. */
> if (!v)
> continue;
> - t = find (v->id);
> - tmp = get_varinfo (t)->solution;
>
> - if (add_graph_edge (graph, t, rhs))
> + if (v->may_have_pointers)
> {
> - if (bitmap_ior_into (get_varinfo (t)->solution, sol))
> + t = find (v->id);
> + if (add_graph_edge (graph, t, rhs))
> {
> - if (t == rhs)
> - sol = get_varinfo (rhs)->solution;
> - if (!TEST_BIT (changed, t))
> - {
> - SET_BIT (changed, t);
> - changed_count++;
> - }
> + if (bitmap_ior_into (get_varinfo (t)->solution, sol))
> + {
> + if (t == rhs)
> + sol = get_varinfo (rhs)->solution;
> + if (!TEST_BIT (changed, t))
> + {
> + SET_BIT (changed, t);
> + changed_count++;
> + }
> + }
> }
> }
> }
> @@ -2751,19 +2756,27 @@ process_constraint (constraint_t t)
> }
> }
>
> +/* Return true if T is a type that could contain pointers. */
> +
> +static bool
> +type_could_have_pointers (tree type)
> +{
> + if (POINTER_TYPE_P (type))
> + return true;
> +
> + if (TREE_CODE (type) == ARRAY_TYPE)
> + return type_could_have_pointers (TREE_TYPE (type));
> +
> + return AGGREGATE_TYPE_P (type);
> +}
> +
> /* Return true if T is a variable of a type that could contain
> pointers. */
>
> static bool
> could_have_pointers (tree t)
> {
> - tree type = TREE_TYPE (t);
> -
> - if (POINTER_TYPE_P (type)
> - || AGGREGATE_TYPE_P (type))
> - return true;
> -
> - return false;
> + return type_could_have_pointers (TREE_TYPE (t));
> }
>
> /* Return the position, in bits, of FIELD_DECL from the beginning of its
> @@ -3527,6 +3540,9 @@ handle_lhs_call (tree lhs, int flags)
> vi = get_varinfo (rhsc.var);
> vi->is_artificial_var = 1;
> vi->is_heap_var = 1;
> + vi->is_unknown_size_var = true;
> + vi->fullsize = ~0;
> + vi->size = ~0;
> rhsc.type = ADDRESSOF;
> rhsc.offset = 0;
> }
> @@ -4367,6 +4383,7 @@ create_variable_info_for (tree decl, con
> vi = new_var_info (decl, index, name);
> vi->decl = decl;
> vi->offset = 0;
> + vi->may_have_pointers = could_have_pointers (decl);
> if (!declsize
> || !host_integerp (declsize, 1))
> {
> @@ -4383,7 +4400,7 @@ create_variable_info_for (tree decl, con
> insert_vi_for_tree (vi->decl, vi);
> VEC_safe_push (varinfo_t, heap, varmap, vi);
> if (is_global && (!flag_whole_program || !in_ipa_mode)
> - && could_have_pointers (decl))
> + && vi->may_have_pointers)
> {
> if (var_ann (decl)
> && var_ann (decl)->noalias_state == NO_ALIAS_ANYTHING)
> @@ -4444,6 +4461,7 @@ create_variable_info_for (tree decl, con
>
> vi->size = fo->size;
> vi->offset = fo->offset;
> + vi->may_have_pointers = fo->may_have_pointers;
> for (i = VEC_length (fieldoff_s, fieldstack) - 1;
> i >= 1 && VEC_iterate (fieldoff_s, fieldstack, i, fo);
> i--)
> @@ -4465,10 +4483,11 @@ create_variable_info_for (tree decl, con
> newvi->offset = fo->offset;
> newvi->size = fo->size;
> newvi->fullsize = vi->fullsize;
> + newvi->may_have_pointers = fo->may_have_pointers;
> insert_into_field_list (vi, newvi);
> VEC_safe_push (varinfo_t, heap, varmap, newvi);
> if (is_global && (!flag_whole_program || !in_ipa_mode)
> - && fo->may_have_pointers)
> + && newvi->may_have_pointers)
> make_constraint_from (newvi, escaped_id);
>
> stats.total_vars++;
> @@ -4552,7 +4571,7 @@ intra_create_variable_infos (void)
> if (heapvar == NULL_TREE)
> {
> var_ann_t ann;
> - heapvar = create_tmp_var_raw (TREE_TYPE (TREE_TYPE (t)),
> + heapvar = create_tmp_var_raw (ptr_type_node,
> "PARM_NOALIAS");
> DECL_EXTERNAL (heapvar) = 1;
> if (gimple_referenced_vars (cfun))
> @@ -4575,6 +4594,9 @@ intra_create_variable_infos (void)
> vi = get_vi_for_tree (heapvar);
> vi->is_artificial_var = 1;
> vi->is_heap_var = 1;
> + vi->is_unknown_size_var = true;
> + vi->fullsize = ~0;
> + vi->size = ~0;
> rhs.var = vi->id;
> rhs.type = ADDRESSOF;
> rhs.offset = 0;
>