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 <ja...@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