This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix PR38151 and a load of alias problems
- From: Richard Guenther <rguenther at suse dot de>
- To: gcc-patches at gcc dot gnu dot org
- Cc: Daniel Berlin <dberlin at dberlin dot org>, Diego Novillo <dnovillo at google dot com>
- Date: Sun, 23 Nov 2008 02:07:22 +0100 (CET)
- Subject: [PATCH] Fix PR38151 and a load of alias problems
This fixes PR38151 by properly using a ref-all pointer in
ix86_gimplify_va_arg to write to the temporary that is then
read as a structure.
This bug exposes several bugs in the way we handle type-based
aliasing with ref-all and DECL_POINTER_ALIAS_SET pointers and
also with setting up the aliases for SMTs. First we shouldn't
do TBAA pruning for those pointers, second we need to build
proper SMTs for such pointers, not only looking at their types
alias-set. Third the optimization we have in place to optimize
the number of aliases for an SMT misses an important fact that
gets exposed with this testcase - we do not account for writes
to parts of a structure which will use a separate SMT.
I actually tested a different patch which makes all accesses
produced by ix86_gimplify_va_arg ref-all which doesn't need the
third bug fixed. This is the second patch after the first one
which is still in testing.
I believe the first patch is correct and necessary - any other
opinions?
Thanks,
Richard.
2008-11-22 Richard Guenther <rguenther@suse.de>
PR middle-end/38151
* tree-ssa-alias.c (struct alias_info): Remove written_vars.
Remove dereferenced_ptrs_store and dereferenced_ptrs_load
in favor of dereferenced_ptrs.
(init_alias_info): Adjust.
(delete_alias_info): Likewise.
(compute_flow_insensitive_aliasing): Properly
include all aliased variables.
(update_alias_info_1): Use dereferenced_ptrs.
(setup_pointers_and_addressables): Likewise.
(get_smt_for): Honor ref-all pointers and pointers with known alias
set properly.
* tree-ssa-structalias.c (find_what_p_points_to): Do not prune
points-to sets for ref-all pointers or pointers with known alias set.
* config/i386/i386.c (ix86_gimplify_va_arg): Use ref-all pointers.
* gcc.c-torture/execute/pr38151.c: New testcase.
Index: gcc/tree-ssa-alias.c
===================================================================
*** gcc/tree-ssa-alias.c (revision 142124)
--- gcc/tree-ssa-alias.c (working copy)
*************** struct alias_info
*** 188,202 ****
struct alias_map_d **pointers;
size_t num_pointers;
! /* Variables that have been written to directly (i.e., not through a
! pointer dereference). */
! struct pointer_set_t *written_vars;
!
! /* Pointers that have been used in an indirect store operation. */
! struct pointer_set_t *dereferenced_ptrs_store;
!
! /* Pointers that have been used in an indirect load operation. */
! struct pointer_set_t *dereferenced_ptrs_load;
};
--- 188,195 ----
struct alias_map_d **pointers;
size_t num_pointers;
! /* Pointers that have been used in an indirect load/store operation. */
! struct pointer_set_t *dereferenced_ptrs;
};
*************** init_alias_info (void)
*** 2073,2081 ****
ai->ssa_names_visited = sbitmap_alloc (num_ssa_names);
sbitmap_zero (ai->ssa_names_visited);
ai->processed_ptrs = VEC_alloc (tree, heap, 50);
! ai->written_vars = pointer_set_create ();
! ai->dereferenced_ptrs_store = pointer_set_create ();
! ai->dereferenced_ptrs_load = pointer_set_create ();
/* Clear out all memory reference stats. */
init_mem_ref_stats ();
--- 2066,2072 ----
ai->ssa_names_visited = sbitmap_alloc (num_ssa_names);
sbitmap_zero (ai->ssa_names_visited);
ai->processed_ptrs = VEC_alloc (tree, heap, 50);
! ai->dereferenced_ptrs = pointer_set_create ();
/* Clear out all memory reference stats. */
init_mem_ref_stats ();
*************** delete_alias_info (struct alias_info *ai
*** 2123,2131 ****
free (ai->pointers[i]);
free (ai->pointers);
! pointer_set_destroy (ai->written_vars);
! pointer_set_destroy (ai->dereferenced_ptrs_store);
! pointer_set_destroy (ai->dereferenced_ptrs_load);
free (ai);
delete_mem_ref_stats (cfun);
--- 2114,2120 ----
free (ai->pointers[i]);
free (ai->pointers);
! pointer_set_destroy (ai->dereferenced_ptrs);
free (ai);
delete_mem_ref_stats (cfun);
*************** compute_flow_insensitive_aliasing (struc
*** 2361,2383 ****
{
struct alias_map_d *v_map;
var_ann_t v_ann;
- bool tag_stored_p, var_stored_p;
v_map = ai->addressable_vars[j];
var = v_map->var;
v_ann = var_ann (var);
! /* Skip memory tags and variables that have never been
! written to. We also need to check if the variables are
! call-clobbered because they may be overwritten by
! function calls. */
! tag_stored_p = pointer_set_contains (ai->written_vars, tag)
! || is_call_clobbered (tag);
! var_stored_p = pointer_set_contains (ai->written_vars, var)
! || is_call_clobbered (var);
! if (!tag_stored_p && !var_stored_p)
! continue;
!
if (may_alias_p (p_map->var, p_map->set, var, v_map->set, false))
{
/* Add VAR to TAG's may-aliases set. */
--- 2350,2367 ----
{
struct alias_map_d *v_map;
var_ann_t v_ann;
v_map = ai->addressable_vars[j];
var = v_map->var;
v_ann = var_ann (var);
! /* We used to skip variables that have never been written to
! if the memory tag has been never written to directly (or
! either of them were call clobbered). This is not enough
! though, as this misses writes through the tags aliases.
! So, for correctness we need to include any aliased
! variable here. */
!
if (may_alias_p (p_map->var, p_map->set, var, v_map->set, false))
{
/* Add VAR to TAG's may-aliases set. */
*************** update_alias_info_1 (gimple stmt, struct
*** 2618,2630 ****
/* ??? For always executed direct dereferences we can
apply TBAA-pruning to their escape set. */
! /* If this is a store operation, mark OP as being
! dereferenced to store, otherwise mark it as being
! dereferenced to load. */
! if (num_stores > 0)
! pointer_set_insert (ai->dereferenced_ptrs_store, var);
! else
! pointer_set_insert (ai->dereferenced_ptrs_load, var);
/* Update the frequency estimate for all the dereferences of
pointer OP. */
--- 2602,2609 ----
/* ??? For always executed direct dereferences we can
apply TBAA-pruning to their escape set. */
! /* Mark OP as being dereferenced. */
! pointer_set_insert (ai->dereferenced_ptrs, var);
/* Update the frequency estimate for all the dereferences of
pointer OP. */
*************** update_alias_info_1 (gimple stmt, struct
*** 2649,2655 ****
if (is_gimple_call (stmt)
|| stmt_escape_type == ESCAPE_STORED_IN_GLOBAL)
{
! pointer_set_insert (ai->dereferenced_ptrs_store, var);
pi->memory_tag_needed = 1;
}
}
--- 2628,2634 ----
if (is_gimple_call (stmt)
|| stmt_escape_type == ESCAPE_STORED_IN_GLOBAL)
{
! pointer_set_insert (ai->dereferenced_ptrs, var);
pi->memory_tag_needed = 1;
}
}
*************** update_alias_info_1 (gimple stmt, struct
*** 2667,2683 ****
mem_ref_stats->num_mem_stmts++;
- /* Add all decls written to to the list of written variables. */
- if (gimple_has_lhs (stmt)
- && TREE_CODE (gimple_get_lhs (stmt)) != SSA_NAME)
- {
- tree lhs = gimple_get_lhs (stmt);
- while (handled_component_p (lhs))
- lhs = TREE_OPERAND (lhs, 0);
- if (DECL_P (lhs))
- pointer_set_insert (ai->written_vars, lhs);
- }
-
/* Notice that we only update memory reference stats for symbols
loaded and stored by the statement if the statement does not
contain pointer dereferences and it is not a call/asm site.
--- 2646,2651 ----
*************** setup_pointers_and_addressables (struct
*** 2770,2776 ****
/* Since we don't keep track of volatile variables, assume that
these pointers are used in indirect store operations. */
if (TREE_THIS_VOLATILE (var))
! pointer_set_insert (ai->dereferenced_ptrs_store, var);
num_pointers++;
}
--- 2738,2744 ----
/* Since we don't keep track of volatile variables, assume that
these pointers are used in indirect store operations. */
if (TREE_THIS_VOLATILE (var))
! pointer_set_insert (ai->dereferenced_ptrs, var);
num_pointers++;
}
*************** setup_pointers_and_addressables (struct
*** 2845,2852 ****
array and create a symbol memory tag for them. */
if (POINTER_TYPE_P (TREE_TYPE (var)))
{
! if ((pointer_set_contains (ai->dereferenced_ptrs_store, var)
! || pointer_set_contains (ai->dereferenced_ptrs_load, var)))
{
tree tag, old_tag;
var_ann_t t_ann;
--- 2813,2819 ----
array and create a symbol memory tag for them. */
if (POINTER_TYPE_P (TREE_TYPE (var)))
{
! if (pointer_set_contains (ai->dereferenced_ptrs, var))
{
tree tag, old_tag;
var_ann_t t_ann;
*************** setup_pointers_and_addressables (struct
*** 2872,2882 ****
/* Associate the tag with pointer VAR. */
set_symbol_mem_tag (var, tag);
-
- /* If pointer VAR has been used in a store operation,
- then its memory tag must be marked as written-to. */
- if (pointer_set_contains (ai->dereferenced_ptrs_store, var))
- pointer_set_insert (ai->written_vars, tag);
}
else
{
--- 2839,2844 ----
*************** get_smt_for (tree ptr, struct alias_info
*** 3298,3304 ****
size_t i;
tree tag;
tree tag_type = TREE_TYPE (TREE_TYPE (ptr));
! alias_set_type tag_set = get_alias_set (tag_type);
/* To avoid creating unnecessary memory tags, only create one memory tag
per alias set class. Note that it may be tempting to group
--- 3260,3281 ----
size_t i;
tree tag;
tree tag_type = TREE_TYPE (TREE_TYPE (ptr));
! alias_set_type tag_set;
!
! /* Get the alias set to be used for the pointed-to memory. If that
! differs from what we would get from looking at the type adjust
! the tag_type to void to make sure we get a proper alias set from
! just looking at the SMT we create. */
! tag_set = get_alias_set (tag_type);
! if (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (ptr))
! /* This is overly conservative but we do not want to assign
! restrict alias sets here (which if they are not assigned
! are -2 but still "known"). */
! || DECL_POINTER_ALIAS_SET_KNOWN_P (ptr))
! {
! tag_set = 0;
! tag_type = void_type_node;
! }
/* To avoid creating unnecessary memory tags, only create one memory tag
per alias set class. Note that it may be tempting to group
*************** get_smt_for (tree ptr, struct alias_info
*** 3329,3335 ****
artificial variable representing the memory location
pointed-to by PTR. */
tag = symbol_mem_tag (ptr);
! if (tag == NULL_TREE)
tag = create_memory_tag (tag_type, true);
/* Add PTR to the POINTERS array. Note that we are not interested in
--- 3306,3313 ----
artificial variable representing the memory location
pointed-to by PTR. */
tag = symbol_mem_tag (ptr);
! if (tag == NULL_TREE
! || tag_set != get_alias_set (tag))
tag = create_memory_tag (tag_type, true);
/* Add PTR to the POINTERS array. Note that we are not interested in
Index: gcc/tree-ssa-structalias.c
===================================================================
*** gcc/tree-ssa-structalias.c (revision 142124)
--- gcc/tree-ssa-structalias.c (working copy)
*************** find_what_p_points_to (tree p)
*** 4817,4823 ****
set_uids_in_ptset (p, finished_solution, vi->solution,
pi->is_dereferenced,
! vi->no_tbaa_pruning);
result = shared_bitmap_lookup (finished_solution);
if (!result)
--- 4817,4825 ----
set_uids_in_ptset (p, finished_solution, vi->solution,
pi->is_dereferenced,
! vi->no_tbaa_pruning
! || TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (p))
! || DECL_POINTER_ALIAS_SET_KNOWN_P (SSA_NAME_VAR (p)));
result = shared_bitmap_lookup (finished_solution);
if (!result)
Index: gcc/config/i386/i386.c
===================================================================
*** gcc/config/i386/i386.c (revision 142124)
--- gcc/config/i386/i386.c (working copy)
*************** ix86_gimplify_va_arg (tree valist, tree
*** 6753,6758 ****
--- 6753,6760 ----
enum machine_mode mode = GET_MODE (reg);
tree piece_type = lang_hooks.types.type_for_mode (mode, 1);
tree addr_type = build_pointer_type (piece_type);
+ tree daddr_type = build_pointer_type_for_mode (piece_type,
+ ptr_mode, true);
tree src_addr, src;
int src_offset;
tree dest_addr, dest;
*************** ix86_gimplify_va_arg (tree valist, tree
*** 6772,6779 ****
size_int (src_offset));
src = build_va_arg_indirect_ref (src_addr);
! dest_addr = fold_convert (addr_type, addr);
! dest_addr = fold_build2 (POINTER_PLUS_EXPR, addr_type, dest_addr,
size_int (INTVAL (XEXP (slot, 1))));
dest = build_va_arg_indirect_ref (dest_addr);
--- 6774,6781 ----
size_int (src_offset));
src = build_va_arg_indirect_ref (src_addr);
! dest_addr = fold_convert (daddr_type, addr);
! dest_addr = fold_build2 (POINTER_PLUS_EXPR, daddr_type, dest_addr,
size_int (INTVAL (XEXP (slot, 1))));
dest = build_va_arg_indirect_ref (dest_addr);
Index: gcc/testsuite/gcc.c-torture/execute/pr38151.c
===================================================================
*** gcc/testsuite/gcc.c-torture/execute/pr38151.c (revision 0)
--- gcc/testsuite/gcc.c-torture/execute/pr38151.c (revision 0)
***************
*** 0 ****
--- 1,46 ----
+ void abort (void);
+
+ struct S2848
+ {
+ unsigned int a;
+ _Complex int b;
+ struct
+ {
+ } __attribute__ ((aligned)) c;
+ };
+
+ struct S2848 s2848;
+
+ int fails;
+
+ void __attribute__((noinline))
+ check2848va (int z, ...)
+ {
+ struct S2848 arg;
+ __builtin_va_list ap;
+
+ __builtin_va_start (ap, z);
+
+ arg = __builtin_va_arg (ap, struct S2848);
+
+ if (s2848.a != arg.a)
+ ++fails;
+ if (s2848.b != arg.b)
+ ++fails;
+
+ __builtin_va_end (ap);
+ }
+
+ int main (void)
+ {
+ s2848.a = 4027477739U;
+ s2848.b = (723419448 + -218144346 * __extension__ 1i);
+
+ check2848va (1, s2848);
+
+ if (fails)
+ abort ();
+
+ return 0;
+ }
+
2008-11-22 Richard Guenther <rguenther@suse.de>
PR middle-end/38151
* tree-ssa-alias.c (get_smt_for): Honor ref-all pointers and
pointers with known alias set properly.
* tree-ssa-structalias.c (find_what_p_points_to): Do not prune
points-to sets for ref-all pointers or pointers with known alias set.
* config/i386/i386.c (ix86_gimplify_va_arg): Use ref-all pointers.
* gcc.c-torture/execute/pr38151.c: New testcase.
Index: gcc/tree-ssa-structalias.c
===================================================================
*** gcc/tree-ssa-structalias.c (revision 142124)
--- gcc/tree-ssa-structalias.c (working copy)
*************** find_what_p_points_to (tree p)
*** 4817,4823 ****
set_uids_in_ptset (p, finished_solution, vi->solution,
pi->is_dereferenced,
! vi->no_tbaa_pruning);
result = shared_bitmap_lookup (finished_solution);
if (!result)
--- 4817,4825 ----
set_uids_in_ptset (p, finished_solution, vi->solution,
pi->is_dereferenced,
! vi->no_tbaa_pruning
! || TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (p))
! || DECL_POINTER_ALIAS_SET_KNOWN_P (SSA_NAME_VAR (p)));
result = shared_bitmap_lookup (finished_solution);
if (!result)
Index: gcc/config/i386/i386.c
===================================================================
*** gcc/config/i386/i386.c (revision 142124)
--- gcc/config/i386/i386.c (working copy)
*************** ix86_gimplify_va_arg (tree valist, tree
*** 6752,6758 ****
rtx reg = XEXP (slot, 0);
enum machine_mode mode = GET_MODE (reg);
tree piece_type = lang_hooks.types.type_for_mode (mode, 1);
! tree addr_type = build_pointer_type (piece_type);
tree src_addr, src;
int src_offset;
tree dest_addr, dest;
--- 6752,6759 ----
rtx reg = XEXP (slot, 0);
enum machine_mode mode = GET_MODE (reg);
tree piece_type = lang_hooks.types.type_for_mode (mode, 1);
! tree addr_type = build_pointer_type_for_mode (piece_type,
! ptr_mode, true);
tree src_addr, src;
int src_offset;
tree dest_addr, dest;
*************** ix86_gimplify_va_arg (tree valist, tree
*** 6834,6840 ****
if (container)
gimple_seq_add_stmt (pre_p, gimple_build_label (lab_over));
! ptrtype = build_pointer_type (type);
addr = fold_convert (ptrtype, addr);
if (indirect_p)
--- 6835,6841 ----
if (container)
gimple_seq_add_stmt (pre_p, gimple_build_label (lab_over));
! ptrtype = build_pointer_type_for_mode (type, ptr_mode, true);
addr = fold_convert (ptrtype, addr);
if (indirect_p)
Index: gcc/testsuite/gcc.c-torture/execute/pr38151.c
===================================================================
*** gcc/testsuite/gcc.c-torture/execute/pr38151.c (revision 0)
--- gcc/testsuite/gcc.c-torture/execute/pr38151.c (revision 0)
***************
*** 0 ****
--- 1,46 ----
+ void abort (void);
+
+ struct S2848
+ {
+ unsigned int a;
+ _Complex int b;
+ struct
+ {
+ } __attribute__ ((aligned)) c;
+ };
+
+ struct S2848 s2848;
+
+ int fails;
+
+ void __attribute__((noinline))
+ check2848va (int z, ...)
+ {
+ struct S2848 arg;
+ __builtin_va_list ap;
+
+ __builtin_va_start (ap, z);
+
+ arg = __builtin_va_arg (ap, struct S2848);
+
+ if (s2848.a != arg.a)
+ ++fails;
+ if (s2848.b != arg.b)
+ ++fails;
+
+ __builtin_va_end (ap);
+ }
+
+ int main (void)
+ {
+ s2848.a = 4027477739U;
+ s2848.b = (723419448 + -218144346 * __extension__ 1i);
+
+ check2848va (1, s2848);
+
+ if (fails)
+ abort ();
+
+ return 0;
+ }
+
Index: gcc/tree-ssa-alias.c
===================================================================
*** gcc/tree-ssa-alias.c (revision 142124)
--- gcc/tree-ssa-alias.c (working copy)
*************** get_smt_for (tree ptr, struct alias_info
*** 3298,3304 ****
size_t i;
tree tag;
tree tag_type = TREE_TYPE (TREE_TYPE (ptr));
! alias_set_type tag_set = get_alias_set (tag_type);
/* To avoid creating unnecessary memory tags, only create one memory tag
per alias set class. Note that it may be tempting to group
--- 3298,3319 ----
size_t i;
tree tag;
tree tag_type = TREE_TYPE (TREE_TYPE (ptr));
! alias_set_type tag_set;
!
! /* Get the alias set to be used for the pointed-to memory. If that
! differs from what we would get from looking at the type adjust
! the tag_type to void to make sure we get a proper alias set from
! just looking at the SMT we create. */
! tag_set = get_alias_set (tag_type);
! if (TYPE_REF_CAN_ALIAS_ALL (TREE_TYPE (ptr))
! /* This is overly conservative but we do not want to assign
! restrict alias sets here (which if they are not assigned
! are -2 but still "known"). */
! || DECL_POINTER_ALIAS_SET_KNOWN_P (ptr))
! {
! tag_set = 0;
! tag_type = void_type_node;
! }
/* To avoid creating unnecessary memory tags, only create one memory tag
per alias set class. Note that it may be tempting to group
*************** get_smt_for (tree ptr, struct alias_info
*** 3329,3335 ****
artificial variable representing the memory location
pointed-to by PTR. */
tag = symbol_mem_tag (ptr);
! if (tag == NULL_TREE)
tag = create_memory_tag (tag_type, true);
/* Add PTR to the POINTERS array. Note that we are not interested in
--- 3344,3351 ----
artificial variable representing the memory location
pointed-to by PTR. */
tag = symbol_mem_tag (ptr);
! if (tag == NULL_TREE
! || tag_set != get_alias_set (tag))
tag = create_memory_tag (tag_type, true);
/* Add PTR to the POINTERS array. Note that we are not interested in