Commit 3d6fd7ce by Richard Biener

tree-optimization/93946 - fix bogus redundant store removal in FRE, DSE and DOM

This fixes a common mistake in removing a store that looks redudnant but
is not because it changes the dynamic type of the memory and thus makes
a difference for following loads with TBAA.

2020-03-03  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/93946
	* alias.h (refs_same_for_tbaa_p): Declare.
	* alias.c (refs_same_for_tbaa_p): New function.
	* tree-ssa-alias.c (ao_ref_alias_set): For a NULL ref return
	zero.
	* tree-ssa-scopedtables.h
	(avail_exprs_stack::lookup_avail_expr): Add output argument
	giving access to the hashtable entry.
	* tree-ssa-scopedtables.c (avail_exprs_stack::lookup_avail_expr):
	Likewise.
	* tree-ssa-dom.c: Include alias.h.
	(dom_opt_dom_walker::optimize_stmt): Validate TBAA state before
	removing redundant store.
	* tree-ssa-sccvn.h (vn_reference_s::base_set): New member.
	(ao_ref_init_from_vn_reference): Adjust prototype.
	(vn_reference_lookup_pieces): Likewise.
	(vn_reference_insert_pieces): Likewise.
	* tree-ssa-sccvn.c: Track base alias set in addition to alias
	set everywhere.
	(eliminate_dom_walker::eliminate_stmt): Also check base alias
	set when removing redundant stores.
	(visit_reference_op_store): Likewise.
	* dse.c (record_store): Adjust valdity check for redundant
	store removal.

	* gcc.dg/torture/pr93946-1.c: New testcase.
	* gcc.dg/torture/pr93946-2.c: Likewise.
parent 01eb1bb0
2020-03-03 Richard Biener <rguenther@suse.de>
PR tree-optimization/93946
* alias.h (refs_same_for_tbaa_p): Declare.
* alias.c (refs_same_for_tbaa_p): New function.
* tree-ssa-alias.c (ao_ref_alias_set): For a NULL ref return
zero.
* tree-ssa-scopedtables.h
(avail_exprs_stack::lookup_avail_expr): Add output argument
giving access to the hashtable entry.
* tree-ssa-scopedtables.c (avail_exprs_stack::lookup_avail_expr):
Likewise.
* tree-ssa-dom.c: Include alias.h.
(dom_opt_dom_walker::optimize_stmt): Validate TBAA state before
removing redundant store.
* tree-ssa-sccvn.h (vn_reference_s::base_set): New member.
(ao_ref_init_from_vn_reference): Adjust prototype.
(vn_reference_lookup_pieces): Likewise.
(vn_reference_insert_pieces): Likewise.
* tree-ssa-sccvn.c: Track base alias set in addition to alias
set everywhere.
(eliminate_dom_walker::eliminate_stmt): Also check base alias
set when removing redundant stores.
(visit_reference_op_store): Likewise.
* dse.c (record_store): Adjust valdity check for redundant
store removal.
2020-03-03 Jakub Jelinek <jakub@redhat.com>
PR target/26877
......
......@@ -368,6 +368,26 @@ rtx_refs_may_alias_p (const_rtx x, const_rtx mem, bool tbaa_p)
&& MEM_ALIAS_SET (mem) != 0);
}
/* Return true if the ref EARLIER behaves the same as LATER with respect
to TBAA for every memory reference that might follow LATER. */
bool
refs_same_for_tbaa_p (tree earlier, tree later)
{
ao_ref earlier_ref, later_ref;
ao_ref_init (&earlier_ref, earlier);
ao_ref_init (&later_ref, later);
alias_set_type earlier_set = ao_ref_alias_set (&earlier_ref);
alias_set_type later_set = ao_ref_alias_set (&later_ref);
if (!(earlier_set == later_set
|| alias_set_subset_of (later_set, earlier_set)))
return false;
alias_set_type later_base_set = ao_ref_base_alias_set (&later_ref);
alias_set_type earlier_base_set = ao_ref_base_alias_set (&earlier_ref);
return (earlier_base_set == later_base_set
|| alias_set_subset_of (later_base_set, earlier_base_set));
}
/* Returns a pointer to the alias set entry for ALIAS_SET, if there is
such an entry, or NULL otherwise. */
......
......@@ -38,6 +38,7 @@ extern void dump_alias_stats_in_alias_c (FILE *s);
tree reference_alias_ptr_type (tree);
bool alias_ptr_types_compatible_p (tree, tree);
int compare_base_decls (tree, tree);
bool refs_same_for_tbaa_p (tree, tree);
/* This alias set can be used to force a memory to conflict with all
other memories, creating a barrier across which no memory reference
......
......@@ -1540,9 +1540,12 @@ record_store (rtx body, bb_info_t bb_info)
width)
/* We can only remove the later store if the earlier aliases
at least all accesses the later one. */
&& (MEM_ALIAS_SET (mem) == MEM_ALIAS_SET (s_info->mem)
&& ((MEM_ALIAS_SET (mem) == MEM_ALIAS_SET (s_info->mem)
|| alias_set_subset_of (MEM_ALIAS_SET (mem),
MEM_ALIAS_SET (s_info->mem))))
MEM_ALIAS_SET (s_info->mem)))
&& (!MEM_EXPR (s_info->mem)
|| refs_same_for_tbaa_p (MEM_EXPR (s_info->mem),
MEM_EXPR (mem)))))
{
if (GET_MODE (mem) == BLKmode)
{
......
2020-03-03 Richard Biener <rguenther@suse.de>
PR tree-optimization/93946
* gcc.dg/torture/pr93946-1.c: New testcase.
* gcc.dg/torture/pr93946-2.c: Likewise.
2020-03-03 Jakub Jelinek <jakub@redhat.com>
PR rtl-optimization/94002
......
/* { dg-do run } */
union U { long long i; long f; };
struct a {union U u;};
struct aa {struct a a;};
struct b {union U u;};
struct bb {struct b b;};
long __attribute__((noipa))
foo (struct bb *bv, void *ptr)
{
struct aa *a = ptr;
struct bb *b = ptr;
bv->b.u.f = 1;
a->a.u.i = 0;
b->b.u.f = 0;
return bv->b.u.f;
}
int
main ()
{
union C {struct aa aa; struct bb bb;} v;
if (foo (&v.bb, &v) != 0)
__builtin_abort ();
return 0;
}
/* { dg-do run } */
/* { dg-additional-options "-fno-tree-dse" } */
union U { long long i; long f; };
struct a {union U u;};
struct aa {struct a a;};
struct b {union U u;};
struct bb {struct b b;};
long __attribute__((noipa))
foo (struct bb *bv, void *ptr)
{
struct aa *a = ptr;
struct bb *b = ptr;
bv->b.u.f = 1;
a->a.u.i = 0;
b->b.u.f = 0;
return bv->b.u.f;
}
int
main ()
{
union C {struct aa aa; struct bb bb;} v;
if (foo (&v.bb, &v) != 0)
__builtin_abort ();
return 0;
}
......@@ -697,6 +697,8 @@ ao_ref_alias_set (ao_ref *ref)
{
if (ref->ref_alias_set != -1)
return ref->ref_alias_set;
if (!ref->ref)
return 0;
ref->ref_alias_set = get_alias_set (ref->ref);
return ref->ref_alias_set;
}
......
......@@ -49,6 +49,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-vrp.h"
#include "vr-values.h"
#include "gimple-ssa-evrp-analyze.h"
#include "alias.h"
/* This file implements optimizations on the dominator tree. */
......@@ -2155,9 +2156,14 @@ dom_opt_dom_walker::optimize_stmt (basic_block bb, gimple_stmt_iterator *si,
else
new_stmt = gimple_build_assign (rhs, lhs);
gimple_set_vuse (new_stmt, gimple_vuse (stmt));
expr_hash_elt *elt = NULL;
cached_lhs = m_avail_exprs_stack->lookup_avail_expr (new_stmt, false,
false);
if (cached_lhs && operand_equal_p (rhs, cached_lhs, 0))
false, &elt);
if (cached_lhs
&& operand_equal_p (rhs, cached_lhs, 0)
&& refs_same_for_tbaa_p (elt->expr ()->kind == EXPR_SINGLE
? elt->expr ()->ops.single.rhs
: NULL_TREE, lhs))
{
basic_block bb = gimple_bb (stmt);
unlink_stmt_vdef (stmt);
......
......@@ -658,6 +658,17 @@ dse_optimize_redundant_stores (gimple *stmt)
{
int cnt = 0;
/* TBAA state of STMT, if it is a call it is effectively alias-set zero. */
alias_set_type earlier_set = 0;
alias_set_type earlier_base_set = 0;
if (is_gimple_assign (stmt))
{
ao_ref lhs_ref;
ao_ref_init (&lhs_ref, gimple_assign_lhs (stmt));
earlier_set = ao_ref_alias_set (&lhs_ref);
earlier_base_set = ao_ref_base_alias_set (&lhs_ref);
}
/* We could do something fairly complex and look through PHIs
like DSE_CLASSIFY_STORE, but it doesn't seem to be worth
the effort.
......@@ -698,10 +709,27 @@ dse_optimize_redundant_stores (gimple *stmt)
{
gimple_stmt_iterator gsi = gsi_for_stmt (use_stmt);
if (is_gimple_assign (use_stmt))
{
ao_ref lhs_ref;
ao_ref_init (&lhs_ref, gimple_assign_lhs (use_stmt));
if ((earlier_set == ao_ref_alias_set (&lhs_ref)
|| alias_set_subset_of (ao_ref_alias_set (&lhs_ref),
earlier_set))
&& (earlier_base_set == ao_ref_base_alias_set (&lhs_ref)
|| alias_set_subset_of
(ao_ref_base_alias_set (&lhs_ref),
earlier_base_set)))
delete_dead_or_redundant_assignment (&gsi, "redundant",
need_eh_cleanup);
}
else if (is_gimple_call (use_stmt))
{
if ((earlier_set == 0
|| alias_set_subset_of (0, earlier_set))
&& (earlier_base_set == 0
|| alias_set_subset_of (0, earlier_base_set)))
delete_dead_or_redundant_call (&gsi, "redundant");
}
else
gcc_unreachable ();
}
......
......@@ -1140,7 +1140,8 @@ fully_constant_expression (pre_expr e)
static tree
translate_vuse_through_block (vec<vn_reference_op_s> operands,
alias_set_type set, tree type, tree vuse,
alias_set_type set, alias_set_type base_set,
tree type, tree vuse,
basic_block phiblock,
basic_block block, bool *same_valid)
{
......@@ -1156,7 +1157,8 @@ translate_vuse_through_block (vec<vn_reference_op_s> operands,
return vuse;
unsigned int cnt = param_sccvn_max_alias_queries_per_access;
use_oracle = ao_ref_init_from_vn_reference (&ref, set, type, operands);
use_oracle = ao_ref_init_from_vn_reference (&ref, set, base_set,
type, operands);
/* Use the alias-oracle to find either the PHI node in this block,
the first VUSE used in this block that is equivalent to vuse or
......@@ -1535,7 +1537,8 @@ phi_translate_1 (bitmap_set_t dest,
{
newvuse = translate_vuse_through_block (newoperands.exists ()
? newoperands : operands,
ref->set, ref->type,
ref->set, ref->base_set,
ref->type,
vuse, phiblock, pred,
changed
? NULL : &same_valid);
......@@ -1551,6 +1554,7 @@ phi_translate_1 (bitmap_set_t dest,
unsigned int new_val_id;
tree result = vn_reference_lookup_pieces (newvuse, ref->set,
ref->base_set,
ref->type,
newoperands.exists ()
? newoperands : operands,
......@@ -1608,7 +1612,7 @@ phi_translate_1 (bitmap_set_t dest,
if (!newoperands.exists ())
newoperands = operands.copy ();
newref = vn_reference_insert_pieces (newvuse, ref->set,
ref->type,
ref->base_set, ref->type,
newoperands,
result, new_val_id);
newoperands = vNULL;
......@@ -1827,8 +1831,8 @@ value_dies_in_block_x (pre_expr expr, basic_block block)
/* Init ref only if we really need it. */
if (ref.base == NULL_TREE
&& !ao_ref_init_from_vn_reference (&ref, refx->set, refx->type,
refx->operands))
&& !ao_ref_init_from_vn_reference (&ref, refx->set, refx->base_set,
refx->type, refx->operands))
{
res = true;
break;
......@@ -3914,12 +3918,16 @@ compute_avail (void)
case VN_REFERENCE:
{
tree rhs1 = gimple_assign_rhs1 (stmt);
alias_set_type set = get_alias_set (rhs1);
ao_ref rhs1_ref;
ao_ref_init (&rhs1_ref, rhs1);
alias_set_type set = ao_ref_alias_set (&rhs1_ref);
alias_set_type base_set
= ao_ref_base_alias_set (&rhs1_ref);
vec<vn_reference_op_s> operands
= vn_reference_operands_for_lookup (rhs1);
vn_reference_t ref;
vn_reference_lookup_pieces (gimple_vuse (stmt), set,
TREE_TYPE (rhs1),
base_set, TREE_TYPE (rhs1),
operands, &ref, VN_WALK);
if (!ref)
{
......
......@@ -143,6 +143,7 @@ typedef struct vn_reference_s
hashval_t hashcode;
tree vuse;
alias_set_type set;
alias_set_type base_set;
tree type;
vec<vn_reference_op_s> operands;
tree result;
......@@ -249,17 +250,17 @@ tree vn_nary_op_lookup_pieces (unsigned int, enum tree_code,
tree, tree *, vn_nary_op_t *);
vn_nary_op_t vn_nary_op_insert_pieces (unsigned int, enum tree_code,
tree, tree *, tree, unsigned int);
bool ao_ref_init_from_vn_reference (ao_ref *, alias_set_type, tree,
vec<vn_reference_op_s> );
bool ao_ref_init_from_vn_reference (ao_ref *, alias_set_type, alias_set_type,
tree, vec<vn_reference_op_s> );
vec<vn_reference_op_s> vn_reference_operands_for_lookup (tree);
tree vn_reference_lookup_pieces (tree, alias_set_type, tree,
tree vn_reference_lookup_pieces (tree, alias_set_type, alias_set_type, tree,
vec<vn_reference_op_s> ,
vn_reference_t *, vn_lookup_kind);
tree vn_reference_lookup (tree, tree, vn_lookup_kind, vn_reference_t *, bool,
tree * = NULL);
void vn_reference_lookup_call (gcall *, vn_reference_t *, vn_reference_t);
vn_reference_t vn_reference_insert_pieces (tree, alias_set_type, tree,
vec<vn_reference_op_s> ,
vn_reference_t vn_reference_insert_pieces (tree, alias_set_type, alias_set_type,
tree, vec<vn_reference_op_s>,
tree, unsigned int);
bool vn_nary_op_eq (const_vn_nary_op_t const vno1,
......
......@@ -223,7 +223,8 @@ avail_exprs_stack::simplify_binary_operation (gimple *stmt,
we finish processing this block and its children. */
tree
avail_exprs_stack::lookup_avail_expr (gimple *stmt, bool insert, bool tbaa_p)
avail_exprs_stack::lookup_avail_expr (gimple *stmt, bool insert, bool tbaa_p,
expr_hash_elt **elt)
{
expr_hash_elt **slot;
tree lhs;
......@@ -317,6 +318,8 @@ avail_exprs_stack::lookup_avail_expr (gimple *stmt, bool insert, bool tbaa_p)
/* Extract the LHS of the assignment so that it can be used as the current
definition of another variable. */
lhs = (*slot)->lhs ();
if (elt)
*elt = *slot;
/* Valueize the result. */
if (TREE_CODE (lhs) == SSA_NAME)
......
......@@ -148,7 +148,7 @@ class avail_exprs_stack
/* Lookup and conditionally insert an expression into the table,
recording enough information to unwind as needed. */
tree lookup_avail_expr (gimple *, bool, bool);
tree lookup_avail_expr (gimple *, bool, bool, expr_hash_elt ** = NULL);
void record_cond (cond_equivalence *);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment