public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/users/marxin/heads/opt-parse-enum-sanity)] sccvn: Improve handling of load masked with integer constant [PR93582]
@ 2020-03-17 16:04 Martin Liska
0 siblings, 0 replies; only message in thread
From: Martin Liska @ 2020-03-17 16:04 UTC (permalink / raw)
To: gcc-cvs
https://gcc.gnu.org/g:b07e4e7c7520ca3e798f514dec0711eea2c027be
commit b07e4e7c7520ca3e798f514dec0711eea2c027be
Author: Jakub Jelinek <jakub@redhat.com>
Date: Tue Mar 3 11:24:33 2020 +0100
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.
Diff:
---
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(-)
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 <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
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 <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
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<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;
}
}
}
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_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>,
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2020-03-17 16:04 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-17 16:04 [gcc(refs/users/marxin/heads/opt-parse-enum-sanity)] sccvn: Improve handling of load masked with integer constant [PR93582] Martin Liska
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).