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

Reply via email to