From b07e4e7c7520ca3e798f514dec0711eea2c027be Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Tue, 3 Mar 2020 11:24:33 +0100 Subject: [PATCH] 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 ; _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 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/ChangeLog | 19 ++ gcc/testsuite/ChangeLog | 7 + gcc/testsuite/gcc.c-torture/execute/pr93582.c | 22 +++ gcc/testsuite/gcc.dg/pr93582.c | 57 ++++++ gcc/testsuite/gcc.dg/tree-ssa/pr93582-10.c | 29 +++ gcc/tree-ssa-sccvn.c | 168 ++++++++++++++---- gcc/tree-ssa-sccvn.h | 2 +- 7 files changed, 264 insertions(+), 40 deletions(-) create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr93582.c create mode 100644 gcc/testsuite/gcc.dg/pr93582.c create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr93582-10.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index b764b6152b1..2023cd3c871 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,22 @@ +2020-03-03 Jakub Jelinek + + 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 PR tree-optimization/93946 diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 3990df0166f..7c4c852ad5d 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2020-03-03 Jakub Jelinek + + 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 PR tree-optimization/93946 diff --git a/gcc/testsuite/gcc.c-torture/execute/pr93582.c b/gcc/testsuite/gcc.c-torture/execute/pr93582.c new file mode 100644 index 00000000000..54f58973832 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr93582.c @@ -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; +} diff --git a/gcc/testsuite/gcc.dg/pr93582.c b/gcc/testsuite/gcc.dg/pr93582.c new file mode 100644 index 00000000000..38bf012d0cf --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr93582.c @@ -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); +} diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr93582-10.c b/gcc/testsuite/gcc.dg/tree-ssa/pr93582-10.c new file mode 100644 index 00000000000..43b52bc522a --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr93582-10.c @@ -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; +} diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c index 1015875591c..9853e9fedae 100644 --- a/gcc/tree-ssa-sccvn.c +++ b/gcc/tree-ssa-sccvn.c @@ -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 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 &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 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 (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; } } } diff --git a/gcc/tree-ssa-sccvn.h b/gcc/tree-ssa-sccvn.h index 2042df118d7..d68e7c0ffa3 100644 --- a/gcc/tree-ssa-sccvn.h +++ b/gcc/tree-ssa-sccvn.h @@ -257,7 +257,7 @@ tree vn_reference_lookup_pieces (tree, alias_set_type, alias_set_type, tree, vec , 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, -- 2.30.2