sccvn: Improve handling of load masked with integer constant [PR93582]

As mentioned in the PR and discussed on IRC, the following patch is the
patch that fixes the originally reported issue.
We have there because of the premature bitfield comparison -> BIT_FIELD_REF
optimization:
  s$s4_19 = 0;
  s.s4 = s$s4_19;
  _10 = BIT_FIELD_REF <s, 8, 0>;
  _13 = _10 & 8;
and no other s fields are initialized.  If they would be all initialized with
constants, then my earlier PR93582 bitfield handling patches would handle it
already, but if at least one bit we ignore after the BIT_AND_EXPR masking
is not initialized or is initialized earlier to non-constant, we aren't able
to look through it until combine, which is too late for the warnings on the
dead code.
This patch handles BIT_AND_EXPR where the first operand is a SSA_NAME
initialized with a memory load and second operand is INTEGER_CST, by trying
a partial def lookup after pushing the ranges of 0 bits in the mask as
artificial initializers.  In the above case on little-endian, we push
offset 0 size 3 {} partial def and offset 4 size 4 (the result is unsigned
char) and then perform normal partial def handling.
My initial version of the patch failed miserably during bootstrap, because
data->finish (...) called vn_reference_lookup_or_insert_for_pieces
which I believe tried to remember the masked value rather than real for the
reference, or for failed lookup visit_reference_op_load called
vn_reference_insert.  The following version makes sure we aren't calling
either of those functions in the masked case, as we don't know anything
better about the reference from whatever has been discovered when the load
stmt has been visited, the patch just calls vn_nary_op_insert_stmt on
failure with the lhs (apparently calling it with the INTEGER_CST doesn't
work).

2020-03-03  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/93582
	* tree-ssa-sccvn.h (vn_reference_lookup): Add mask argument.
	* tree-ssa-sccvn.c (struct vn_walk_cb_data): Add mask and masked_result
	members, initialize them in the constructor and if mask is non-NULL,
	artificially push_partial_def {} for the portions of the mask that
	contain zeros.
	(vn_walk_cb_data::finish): If mask is non-NULL, set masked_result to
	val and return (void *)-1.  Formatting fix.
	(vn_reference_lookup_pieces): Adjust vn_walk_cb_data initialization.
	Formatting fix.
	(vn_reference_lookup): Add mask argument.  If non-NULL, don't call
	fully_constant_vn_reference_p nor vn_reference_lookup_1 and return
	data.mask_result.
	(visit_nary_op): Handle BIT_AND_EXPR of a memory load and INTEGER_CST
	mask.
	(visit_stmt): Formatting fix.

	* gcc.dg/tree-ssa/pr93582-10.c: New test.
	* gcc.dg/pr93582.c: New test.
	* gcc.c-torture/execute/pr93582.c: New test.
This commit is contained in:
Jakub Jelinek 2020-03-03 11:24:33 +01:00
parent 3d6fd7ce6d
commit b07e4e7c75
7 changed files with 264 additions and 40 deletions

View file

@ -1,3 +1,22 @@
2020-03-03 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/93582
* tree-ssa-sccvn.h (vn_reference_lookup): Add mask argument.
* tree-ssa-sccvn.c (struct vn_walk_cb_data): Add mask and masked_result
members, initialize them in the constructor and if mask is non-NULL,
artificially push_partial_def {} for the portions of the mask that
contain zeros.
(vn_walk_cb_data::finish): If mask is non-NULL, set masked_result to
val and return (void *)-1. Formatting fix.
(vn_reference_lookup_pieces): Adjust vn_walk_cb_data initialization.
Formatting fix.
(vn_reference_lookup): Add mask argument. If non-NULL, don't call
fully_constant_vn_reference_p nor vn_reference_lookup_1 and return
data.mask_result.
(visit_nary_op): Handle BIT_AND_EXPR of a memory load and INTEGER_CST
mask.
(visit_stmt): Formatting fix.
2020-03-03 Richard Biener <rguenther@suse.de>
PR tree-optimization/93946

View file

@ -1,3 +1,10 @@
2020-03-03 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/93582
* gcc.dg/tree-ssa/pr93582-10.c: New test.
* gcc.dg/pr93582.c: New test.
* gcc.c-torture/execute/pr93582.c: New test.
2020-03-03 Richard Biener <rguenther@suse.de>
PR tree-optimization/93946

View file

@ -0,0 +1,22 @@
/* PR tree-optimization/93582 */
short a;
int b, c;
__attribute__((noipa)) void
foo (void)
{
b = c;
a &= 7;
}
int
main ()
{
c = 27;
a = 14;
foo ();
if (b != 27 || a != 6)
__builtin_abort ();
return 0;
}

View file

@ -0,0 +1,57 @@
/* PR tree-optimization/93582 */
/* { dg-do compile } */
/* { dg-options "-O2 -Warray-bounds" } */
struct S {
unsigned int s1:1;
unsigned int s2:1;
unsigned int s3:1;
unsigned int s4:1;
unsigned int s5:4;
unsigned char s6;
unsigned short s7;
unsigned short s8;
};
struct T {
int t1;
int t2;
};
static inline int
bar (struct S *x)
{
if (x->s4)
return ((struct T *)(x + 1))->t1 + ((struct T *)(x + 1))->t2; /* { dg-bogus "array subscript 1 is outside array bounds of" } */
else
return 0;
}
int
foo (int x, int y)
{
struct S s; /* { dg-bogus "while referencing" } */
s.s6 = x;
s.s7 = y & 0x1FFF;
s.s4 = 0;
return bar (&s);
}
static inline int
qux (struct S *x)
{
int s4 = x->s4;
if (s4)
return ((struct T *)(x + 1))->t1 + ((struct T *)(x + 1))->t2;
else
return 0;
}
int
baz (int x, int y)
{
struct S s;
s.s6 = x;
s.s7 = y & 0x1FFF;
s.s4 = 0;
return qux (&s);
}

View file

@ -0,0 +1,29 @@
/* PR tree-optimization/93582 */
/* { dg-do compile { target int32 } } */
/* { dg-options "-O2 -fdump-tree-fre1" } */
/* { dg-final { scan-tree-dump "return 72876566;" "fre1" { target le } } } */
/* { dg-final { scan-tree-dump "return 559957376;" "fre1" { target be } } } */
union U {
struct S { int a : 12, b : 5, c : 10, d : 5; } s;
unsigned int i;
};
struct A { char a[12]; union U u; };
void bar (struct A *);
unsigned
foo (void)
{
struct A a;
bar (&a);
a.u.s.a = 1590;
a.u.s.c = -404;
#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
#define M 0x67e0a5f
#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
#define M 0xa5f067e0
#else
#define M 0
#endif
return a.u.i & M;
}

View file

@ -1682,16 +1682,55 @@ struct pd_data
struct vn_walk_cb_data
{
vn_walk_cb_data (vn_reference_t vr_, tree orig_ref_, tree *last_vuse_ptr_,
vn_lookup_kind vn_walk_kind_, bool tbaa_p_)
vn_lookup_kind vn_walk_kind_, bool tbaa_p_, tree mask_)
: vr (vr_), last_vuse_ptr (last_vuse_ptr_), last_vuse (NULL_TREE),
vn_walk_kind (vn_walk_kind_), tbaa_p (tbaa_p_),
saved_operands (vNULL), first_set (-2), first_base_set (-2),
known_ranges (NULL)
{
if (!last_vuse_ptr)
last_vuse_ptr = &last_vuse;
ao_ref_init (&orig_ref, orig_ref_);
}
mask (mask_), masked_result (NULL_TREE), vn_walk_kind (vn_walk_kind_),
tbaa_p (tbaa_p_), saved_operands (vNULL), first_set (-2),
first_base_set (-2), known_ranges (NULL)
{
if (!last_vuse_ptr)
last_vuse_ptr = &last_vuse;
ao_ref_init (&orig_ref, orig_ref_);
if (mask)
{
wide_int w = wi::to_wide (mask);
unsigned int pos = 0, prec = w.get_precision ();
pd_data pd;
pd.rhs = build_constructor (NULL_TREE, NULL);
/* When bitwise and with a constant is done on a memory load,
we don't really need all the bits to be defined or defined
to constants, we don't really care what is in the position
corresponding to 0 bits in the mask.
So, push the ranges of those 0 bits in the mask as artificial
zero stores and let the partial def handling code do the
rest. */
while (pos < prec)
{
int tz = wi::ctz (w);
if (pos + tz > prec)
tz = prec - pos;
if (tz)
{
if (BYTES_BIG_ENDIAN)
pd.offset = prec - pos - tz;
else
pd.offset = pos;
pd.size = tz;
void *r = push_partial_def (pd, 0, 0, prec);
gcc_assert (r == NULL_TREE);
}
pos += tz;
if (pos == prec)
break;
w = wi::lrshift (w, tz);
tz = wi::ctz (wi::bit_not (w));
if (pos + tz > prec)
tz = prec - pos;
pos += tz;
w = wi::lrshift (w, tz);
}
}
}
~vn_walk_cb_data ();
void *finish (alias_set_type, alias_set_type, tree);
void *push_partial_def (const pd_data& pd,
@ -1701,6 +1740,8 @@ struct vn_walk_cb_data
ao_ref orig_ref;
tree *last_vuse_ptr;
tree last_vuse;
tree mask;
tree masked_result;
vn_lookup_kind vn_walk_kind;
bool tbaa_p;
vec<vn_reference_op_s> saved_operands;
@ -1733,9 +1774,15 @@ vn_walk_cb_data::finish (alias_set_type set, alias_set_type base_set, tree val)
set = first_set;
base_set = first_base_set;
}
return vn_reference_lookup_or_insert_for_pieces
(last_vuse, set, base_set, vr->type,
saved_operands.exists () ? saved_operands : vr->operands, val);
if (mask)
{
masked_result = val;
return (void *) -1;
}
vec<vn_reference_op_s> &operands
= saved_operands.exists () ? saved_operands : vr->operands;
return vn_reference_lookup_or_insert_for_pieces (last_vuse, set, base_set,
vr->type, operands, val);
}
/* pd_range splay-tree helpers. */
@ -3382,13 +3429,14 @@ vn_reference_lookup_pieces (tree vuse, alias_set_type set,
{
ao_ref r;
unsigned limit = param_sccvn_max_alias_queries_per_access;
vn_walk_cb_data data (&vr1, NULL_TREE, NULL, kind, true);
if (ao_ref_init_from_vn_reference (&r, set, base_set, type, vr1.operands))
*vnresult =
(vn_reference_t)walk_non_aliased_vuses (&r, vr1.vuse, true,
vn_reference_lookup_2,
vn_reference_lookup_3,
vuse_valueize, limit, &data);
vn_walk_cb_data data (&vr1, NULL_TREE, NULL, kind, true, NULL_TREE);
if (ao_ref_init_from_vn_reference (&r, set, base_set, type,
vr1.operands))
*vnresult
= ((vn_reference_t)
walk_non_aliased_vuses (&r, vr1.vuse, true, vn_reference_lookup_2,
vn_reference_lookup_3, vuse_valueize,
limit, &data));
gcc_checking_assert (vr1.operands == shared_lookup_references);
}
@ -3404,15 +3452,19 @@ vn_reference_lookup_pieces (tree vuse, alias_set_type set,
was NULL.. VNRESULT will be filled in with the vn_reference_t
stored in the hashtable if one exists. When TBAA_P is false assume
we are looking up a store and treat it as having alias-set zero.
*LAST_VUSE_PTR will be updated with the VUSE the value lookup succeeded. */
*LAST_VUSE_PTR will be updated with the VUSE the value lookup succeeded.
MASK is either NULL_TREE, or can be an INTEGER_CST if the result of the
load is bitwise anded with MASK and so we are only interested in a subset
of the bits and can ignore if the other bits are uninitialized or
not initialized with constants. */
tree
vn_reference_lookup (tree op, tree vuse, vn_lookup_kind kind,
vn_reference_t *vnresult, bool tbaa_p, tree *last_vuse_ptr)
vn_reference_t *vnresult, bool tbaa_p,
tree *last_vuse_ptr, tree mask)
{
vec<vn_reference_op_s> operands;
struct vn_reference_s vr1;
tree cst;
bool valuezied_anything;
if (vnresult)
@ -3427,11 +3479,11 @@ vn_reference_lookup (tree op, tree vuse, vn_lookup_kind kind,
vr1.set = ao_ref_alias_set (&op_ref);
vr1.base_set = ao_ref_base_alias_set (&op_ref);
vr1.hashcode = vn_reference_compute_hash (&vr1);
if ((cst = fully_constant_vn_reference_p (&vr1)))
return cst;
if (mask == NULL_TREE)
if (tree cst = fully_constant_vn_reference_p (&vr1))
return cst;
if (kind != VN_NOWALK
&& vr1.vuse)
if (kind != VN_NOWALK && vr1.vuse)
{
vn_reference_t wvnresult;
ao_ref r;
@ -3443,25 +3495,31 @@ vn_reference_lookup (tree op, tree vuse, vn_lookup_kind kind,
vr1.type, vr1.operands))
ao_ref_init (&r, op);
vn_walk_cb_data data (&vr1, r.ref ? NULL_TREE : op,
last_vuse_ptr, kind, tbaa_p);
wvnresult =
(vn_reference_t)walk_non_aliased_vuses (&r, vr1.vuse, tbaa_p,
vn_reference_lookup_2,
vn_reference_lookup_3,
vuse_valueize, limit, &data);
last_vuse_ptr, kind, tbaa_p, mask);
wvnresult
= ((vn_reference_t)
walk_non_aliased_vuses (&r, vr1.vuse, tbaa_p, vn_reference_lookup_2,
vn_reference_lookup_3, vuse_valueize, limit,
&data));
gcc_checking_assert (vr1.operands == shared_lookup_references);
if (wvnresult)
{
gcc_assert (mask == NULL_TREE);
if (vnresult)
*vnresult = wvnresult;
return wvnresult->result;
}
else if (mask)
return data.masked_result;
return NULL_TREE;
}
if (last_vuse_ptr)
*last_vuse_ptr = vr1.vuse;
if (mask)
return NULL_TREE;
return vn_reference_lookup_1 (&vr1, vnresult);
}
@ -4675,7 +4733,39 @@ visit_nary_op (tree lhs, gassign *stmt)
}
}
}
default:;
break;
case BIT_AND_EXPR:
if (INTEGRAL_TYPE_P (type)
&& TREE_CODE (rhs1) == SSA_NAME
&& TREE_CODE (gimple_assign_rhs2 (stmt)) == INTEGER_CST
&& !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (rhs1)
&& default_vn_walk_kind != VN_NOWALK
&& CHAR_BIT == 8
&& BITS_PER_UNIT == 8
&& BYTES_BIG_ENDIAN == WORDS_BIG_ENDIAN
&& !integer_all_onesp (gimple_assign_rhs2 (stmt))
&& !integer_zerop (gimple_assign_rhs2 (stmt)))
{
gassign *ass = dyn_cast <gassign *> (SSA_NAME_DEF_STMT (rhs1));
if (ass
&& !gimple_has_volatile_ops (ass)
&& vn_get_stmt_kind (ass) == VN_REFERENCE)
{
tree last_vuse = gimple_vuse (ass);
tree op = gimple_assign_rhs1 (ass);
tree result = vn_reference_lookup (op, gimple_vuse (ass),
default_vn_walk_kind,
NULL, true, &last_vuse,
gimple_assign_rhs2 (stmt));
if (result
&& useless_type_conversion_p (TREE_TYPE (result),
TREE_TYPE (op)))
return set_ssa_val_to (lhs, result);
}
}
break;
default:
break;
}
bool changed = set_ssa_val_to (lhs, lhs);
@ -5192,14 +5282,14 @@ visit_stmt (gimple *stmt, bool backedges_varying_p = false)
switch (vn_get_stmt_kind (ass))
{
case VN_NARY:
changed = visit_nary_op (lhs, ass);
break;
changed = visit_nary_op (lhs, ass);
break;
case VN_REFERENCE:
changed = visit_reference_op_load (lhs, rhs1, ass);
break;
changed = visit_reference_op_load (lhs, rhs1, ass);
break;
default:
changed = defs_to_varying (ass);
break;
changed = defs_to_varying (ass);
break;
}
}
}

View file

@ -257,7 +257,7 @@ 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);
tree * = NULL, tree = NULL_TREE);
void vn_reference_lookup_call (gcall *, vn_reference_t *, vn_reference_t);
vn_reference_t vn_reference_insert_pieces (tree, alias_set_type, alias_set_type,
tree, vec<vn_reference_op_s>,