[PATCH] sccvn: Improve handling of load masked with integer constant [PR93582]
Jakub Jelinek
jakub@redhat.com
Mon Mar 2 14:54:00 GMT 2020
On Mon, Mar 02, 2020 at 12:46:30PM +0100, Richard Biener wrote:
> > + void *r = data.push_partial_def (pd, 0, prec);
> > + if (r == (void *) -1)
> > + return NULL_TREE;
> > + 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);
> > + }
>
> I'd do this in the vn_walk_cb_data CTOR instead - you pass mask !=
> NULL_TREE anyway so you can as well pass mask.
I've tried, but have no idea how to handle the case where
data.push_partial_def (pd, 0, prec); fails above if it is done in the
constructor.
Though, the BIT_AND_EXPR case already checks:
+ && CHAR_BIT == 8
+ && BITS_PER_UNIT == 8
+ && BYTES_BIG_ENDIAN == WORDS_BIG_ENDIAN
and also checks the pathological cases of mask being all ones or all zeros,
so it is just the theoretical case of
maxsizei > bufsize * BITS_PER_UNIT
so maybe it is moot and we can just assert that push_partial_def
returned NULL.
> I wonder if we can instead make the above return NULL (finish
> return (void *)-1) and do sth like
>
> if (!wvnresult && mask)
> return data.masked_result;
>
> and thus avoid the type-"unsafe" return frobbing by storing the
> result value in an extra member of the vn_walk_cb_data struct.
Done that way.
> Any reason you piggy-back on visit_reference_op_load instead of using
> vn_reference_lookup directly? I'd very much prefer that since it
> doesn't even try to mess with the SSA lattice.
I didn't want to duplicate the VCE case, but it isn't that long.
So, like this if it passes bootstrap/regtest?
2020-03-02 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.
--- gcc/tree-ssa-sccvn.h.jj 2020-02-28 17:32:56.391363613 +0100
+++ gcc/tree-ssa-sccvn.h 2020-03-02 13:52:17.488680037 +0100
@@ -256,7 +256,7 @@ tree vn_reference_lookup_pieces (tree, a
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, tree,
vec<vn_reference_op_s> ,
--- gcc/tree-ssa-sccvn.c.jj 2020-02-28 17:32:56.390363628 +0100
+++ gcc/tree-ssa-sccvn.c 2020-03-02 15:48:12.982620557 +0100
@@ -1686,15 +1686,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), 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),
+ 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, 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, tree);
void *push_partial_def (const pd_data& pd, alias_set_type, HOST_WIDE_INT);
@@ -1703,6 +1743,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;
@@ -1731,9 +1773,15 @@ vn_walk_cb_data::finish (alias_set_type
{
if (first_set != -2)
set = first_set;
- return vn_reference_lookup_or_insert_for_pieces
- (last_vuse, 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, vr->type,
+ operands, val);
}
/* pd_range splay-tree helpers. */
@@ -3380,13 +3428,13 @@ vn_reference_lookup_pieces (tree vuse, a
{
ao_ref r;
unsigned limit = param_sccvn_max_alias_queries_per_access;
- vn_walk_cb_data data (&vr1, NULL_TREE, NULL, kind, true);
+ vn_walk_cb_data data (&vr1, NULL_TREE, NULL, kind, true, NULL_TREE);
if (ao_ref_init_from_vn_reference (&r, 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);
+ *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);
}
@@ -3402,15 +3450,19 @@ vn_reference_lookup_pieces (tree vuse, a
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)
@@ -3422,11 +3474,11 @@ vn_reference_lookup (tree op, tree vuse,
vr1.type = TREE_TYPE (op);
vr1.set = get_alias_set (op);
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;
@@ -3438,25 +3490,31 @@ vn_reference_lookup (tree op, tree vuse,
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);
}
@@ -4664,7 +4722,57 @@ 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)
+ {
+ /* We handle type-punning through unions by value-numbering
+ based on offset and size of the access. Be prepared to
+ handle a type-mismatch here via creating a
+ VIEW_CONVERT_EXPR. */
+ if (!useless_type_conversion_p (TREE_TYPE (result),
+ TREE_TYPE (op)))
+ {
+ /* We will be setting the value number of lhs to the
+ value number of
+ VIEW_CONVERT_EXPR <TREE_TYPE (result)> (result).
+ So first simplify and lookup this expression to see
+ if it is already available. */
+ gimple_match_op res_op (gimple_match_cond::UNCOND,
+ VIEW_CONVERT_EXPR,
+ TREE_TYPE (op), result);
+ result = vn_nary_build_or_lookup (&res_op);
+ }
+ }
+ if (result)
+ return set_ssa_val_to (lhs, result);
+ }
+ }
+ break;
+ default:
+ break;
}
bool changed = set_ssa_val_to (lhs, lhs);
@@ -5175,14 +5283,14 @@ visit_stmt (gimple *stmt, bool backedges
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;
}
}
}
--- gcc/testsuite/gcc.dg/tree-ssa/pr93582-10.c.jj 2020-03-02 13:52:17.504679803 +0100
+++ gcc/testsuite/gcc.dg/tree-ssa/pr93582-10.c 2020-03-02 13:52:17.504679803 +0100
@@ -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;
+}
--- gcc/testsuite/gcc.dg/pr93582.c.jj 2020-03-02 13:52:17.504679803 +0100
+++ gcc/testsuite/gcc.dg/pr93582.c 2020-03-02 13:52:17.504679803 +0100
@@ -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);
+}
--- gcc/testsuite/gcc.c-torture/execute/pr93582.c.jj 2020-03-02 13:52:17.504679803 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr93582.c 2020-03-02 13:52:17.504679803 +0100
@@ -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;
+}
Jakub
More information about the Gcc-patches
mailing list