sccvn: Improve handling of load masked with integer constant [PR93582]
authorJakub Jelinek <jakub@redhat.com>
Tue, 3 Mar 2020 10:24:33 +0000 (11:24 +0100)
committerJakub Jelinek <jakub@redhat.com>
Tue, 3 Mar 2020 10:24:33 +0000 (11:24 +0100)
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.

gcc/ChangeLog
gcc/testsuite/ChangeLog
gcc/testsuite/gcc.c-torture/execute/pr93582.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/pr93582.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/tree-ssa/pr93582-10.c [new file with mode: 0644]
gcc/tree-ssa-sccvn.c
gcc/tree-ssa-sccvn.h

index b764b6152b18d65479b198c0e79b7898e36e64ea..2023cd3c871f84cd6f179993cadb8244de8a384d 100644 (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
index 3990df0166f1cb0739ce379f522d50e5cb1df6b8..7c4c852ad5d6ac0f2b1cc7200f88c7400314a41a 100644 (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
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr93582.c b/gcc/testsuite/gcc.c-torture/execute/pr93582.c
new file mode 100644 (file)
index 0000000..54f5897
--- /dev/null
@@ -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 (file)
index 0000000..38bf012
--- /dev/null
@@ -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 (file)
index 0000000..43b52bc
--- /dev/null
@@ -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;
+}
index 1015875591c50b26d9e3a0a55226ba67d24bbe35..9853e9fedaea7be768f1ca9226badadb670152ee 100644 (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;
                }
            }
        }
index 2042df118d7ebe3acbdcf87196cab8d742e9b0d6..d68e7c0ffa314b754e18b818d72c28bf7d518dbe 100644 (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>,