When working on handling partial defs in VN I noticed that
we do not treat MEM[a + 1] and MEM[a + 2] as equal bases.
The following patch fixes that, together with three testcases
illustrating the fixed missed optimizations.

This then caused bogus redundant store removal by FRE again
which made me refactor how we handle its !tbaa_p handling,
fixing a few omissions on the way as well.  Which turned
out to fix PR91091 (see the separate fix for another issue
_that_ triggered...).

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Richard.

2019-07-05  Richard Biener  <rguent...@suse.de>

        PR tree-optimization/91091
        * tree-ssa-alias.h (get_continuation_for_phi): Add tbaa_p parameter.
        (walk_non_aliased_vuses): Likewise.
        * tree-ssa-alias.c (maybe_skip_until): Pass down tbaa_p.
        (get_continuation_for_phi): New tbaa_p parameter and pass
        it down.
        (walk_non_aliased_vuses): Likewise.
        * ipa-prop.c (determine_known_aggregate_parts): Adjust.
        * tree-ssa-pre.c (translate_vuse_through_block): Likewise.
        * tree-ssa-scopedtables.c (avail_exprs_stack::lookup_avail_expr):
        Likewise.
        * tree-ssa-sccvn.c (struct vn_walk_cb_data): Add tbaa_p flag.
        (adjust_offsets_for_equal_base_address): New function.
        (vn_reference_lookup_3): Use it to catch more base equivalences.
        Handle and pass down tbaa_p flag.
        (vn_reference_lookup_pieces): Adjust.
        (vn_reference_lookup): Remove alias-set altering, instead pass
        down false as tbaa_p.

        * gcc.dg/tree-ssa/pr91091-2.c: New testcase.
        * gcc.dg/tree-ssa/ssa-fre-70.c: Likewise.
        * gcc.dg/tree-ssa/ssa-fre-71.c: Likewise.
        * gcc.dg/tree-ssa/ssa-fre-72.c: Likewise.

Index: gcc/ipa-prop.c
===================================================================
--- gcc/ipa-prop.c      (revision 273041)
+++ gcc/ipa-prop.c      (working copy)
@@ -1678,7 +1678,8 @@ determine_known_aggregate_parts (gcall *
 
       if (gimple_code (stmt) == GIMPLE_PHI)
        {
-         dom_vuse = get_continuation_for_phi (stmt, &r, *aa_walk_budget_p,
+         dom_vuse = get_continuation_for_phi (stmt, &r, true,
+                                              *aa_walk_budget_p,
                                               &visited, false, NULL, NULL);
          continue;
        }
Index: gcc/testsuite/gcc.dg/tree-ssa/pr91091-2.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/pr91091-2.c   (nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr91091-2.c   (working copy)
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-fre1" } */
+
+struct s { int x; };
+struct t { int x; };
+
+void swap(struct s* p, struct t* q)
+{
+  p->x = q->x;
+  q->x = p->x;
+}
+
+/* The second statement is redundant.  */
+/* { dg-final { scan-tree-dump-times "x = " 1 "fre1" } } */
+/* { dg-final { scan-tree-dump-times " = \[^;\]*x;" 1 "fre1" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-70.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-70.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-70.c  (working copy)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fgimple -O -fdump-tree-fre1" } */
+
+__GIMPLE (ssa, startwith("fre")) char foo(char *p)
+{
+  char _1;
+
+__BB(2):
+  __MEM <char[4]> (p) = _Literal (char[4]) {};
+  _1 = __MEM <char> (p + 1);
+  return _1;
+}
+
+/* { dg-final { scan-tree-dump "return 0;" "fre1" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-71.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-71.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-71.c  (working copy)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fgimple -O -fdump-tree-fre1-details" } */
+
+__GIMPLE (ssa, startwith("fre")) char foo(char *p)
+{
+  char _1;
+
+__BB(2):
+  __MEM <int> (p) = 0;
+  _1 = __MEM <char> (p + 1);
+  return _1;
+}
+
+/* { dg-final { scan-tree-dump "return 0;" "fre1" } } */
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-72.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-72.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-fre-72.c  (working copy)
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+/* { dg-options "-fgimple -O -fdump-tree-fre1" } */
+
+__GIMPLE (ssa,startwith("fre")) char foo(char *p, int i)
+{
+  char _1;
+
+__BB(2):
+  __MEM <int> (p) = i_2(D);
+  _1 = __MEM <char> (p + 1);
+  return _1;
+}
+
+/* { dg-final { scan-tree-dump "BIT_FIELD_REF" "fre1" } } */
Index: gcc/tree-ssa-alias.c
===================================================================
--- gcc/tree-ssa-alias.c        (revision 273041)
+++ gcc/tree-ssa-alias.c        (working copy)
@@ -3003,8 +3003,8 @@ stmt_kills_ref_p (gimple *stmt, tree ref
 
 static bool
 maybe_skip_until (gimple *phi, tree &target, basic_block target_bb,
-                 ao_ref *ref, tree vuse, unsigned int &limit, bitmap *visited,
-                 bool abort_on_visited,
+                 ao_ref *ref, tree vuse, bool tbaa_p, unsigned int &limit,
+                 bitmap *visited, bool abort_on_visited,
                  void *(*translate)(ao_ref *, tree, void *, bool *),
                  void *data)
 {
@@ -3038,7 +3038,7 @@ maybe_skip_until (gimple *phi, tree &tar
          /* An already visited PHI node ends the walk successfully.  */
          if (bitmap_bit_p (*visited, SSA_NAME_VERSION (PHI_RESULT (def_stmt))))
            return !abort_on_visited;
-         vuse = get_continuation_for_phi (def_stmt, ref, limit,
+         vuse = get_continuation_for_phi (def_stmt, ref, tbaa_p, limit,
                                           visited, abort_on_visited,
                                           translate, data);
          if (!vuse)
@@ -3053,7 +3053,7 @@ maybe_skip_until (gimple *phi, tree &tar
          if ((int)limit <= 0)
            return false;
          --limit;
-         if (stmt_may_clobber_ref_p_1 (def_stmt, ref))
+         if (stmt_may_clobber_ref_p_1 (def_stmt, ref, tbaa_p))
            {
              bool disambiguate_only = true;
              if (translate
@@ -3085,7 +3085,7 @@ maybe_skip_until (gimple *phi, tree &tar
    Returns NULL_TREE if no suitable virtual operand can be found.  */
 
 tree
-get_continuation_for_phi (gimple *phi, ao_ref *ref,
+get_continuation_for_phi (gimple *phi, ao_ref *ref, bool tbaa_p,
                          unsigned int &limit, bitmap *visited,
                          bool abort_on_visited,
                          void *(*translate)(ao_ref *, tree, void *, bool *),
@@ -3128,7 +3128,8 @@ get_continuation_for_phi (gimple *phi, a
       arg1 = PHI_ARG_DEF (phi, i);
       if (arg1 == arg0)
        ;
-      else if (! maybe_skip_until (phi, arg0, dom, ref, arg1, limit, visited,
+      else if (! maybe_skip_until (phi, arg0, dom, ref, arg1, tbaa_p,
+                                  limit, visited,
                                   abort_on_visited,
                                   /* Do not translate when walking over
                                      backedges.  */
@@ -3172,7 +3173,7 @@ get_continuation_for_phi (gimple *phi, a
    TODO: Cache the vector of equivalent vuses per ref, vuse pair.  */
 
 void *
-walk_non_aliased_vuses (ao_ref *ref, tree vuse,
+walk_non_aliased_vuses (ao_ref *ref, tree vuse, bool tbaa_p,
                        void *(*walker)(ao_ref *, tree, void *),
                        void *(*translate)(ao_ref *, tree, void *, bool *),
                        tree (*valueize)(tree),
@@ -3213,7 +3214,7 @@ walk_non_aliased_vuses (ao_ref *ref, tre
       if (gimple_nop_p (def_stmt))
        break;
       else if (gimple_code (def_stmt) == GIMPLE_PHI)
-       vuse = get_continuation_for_phi (def_stmt, ref, limit,
+       vuse = get_continuation_for_phi (def_stmt, ref, tbaa_p, limit,
                                         &visited, translated, translate, data);
       else
        {
@@ -3223,7 +3224,7 @@ walk_non_aliased_vuses (ao_ref *ref, tre
              break;
            }
          --limit;
-         if (stmt_may_clobber_ref_p_1 (def_stmt, ref))
+         if (stmt_may_clobber_ref_p_1 (def_stmt, ref, tbaa_p))
            {
              if (!translate)
                break;
Index: gcc/tree-ssa-alias.h
===================================================================
--- gcc/tree-ssa-alias.h        (revision 273041)
+++ gcc/tree-ssa-alias.h        (working copy)
@@ -131,11 +131,11 @@ extern bool call_may_clobber_ref_p (gcal
 extern bool call_may_clobber_ref_p_1 (gcall *, ao_ref *);
 extern bool stmt_kills_ref_p (gimple *, tree);
 extern bool stmt_kills_ref_p (gimple *, ao_ref *);
-extern tree get_continuation_for_phi (gimple *, ao_ref *,
+extern tree get_continuation_for_phi (gimple *, ao_ref *, bool,
                                      unsigned int &, bitmap *, bool,
                                      void *(*)(ao_ref *, tree, void *, bool *),
                                      void *);
-extern void *walk_non_aliased_vuses (ao_ref *, tree,
+extern void *walk_non_aliased_vuses (ao_ref *, tree, bool,
                                     void *(*)(ao_ref *, tree, void *),
                                     void *(*)(ao_ref *, tree, void *, bool *),
                                     tree (*)(tree), unsigned &, void *);
Index: gcc/tree-ssa-pre.c
===================================================================
--- gcc/tree-ssa-pre.c  (revision 273041)
+++ gcc/tree-ssa-pre.c  (working copy)
@@ -1185,8 +1185,8 @@ translate_vuse_through_block (vec<vn_ref
          bitmap visited = NULL;
          /* Try to find a vuse that dominates this phi node by skipping
             non-clobbering statements.  */
-         vuse = get_continuation_for_phi (phi, &ref, cnt, &visited, false,
-                                          NULL, NULL);
+         vuse = get_continuation_for_phi (phi, &ref, true,
+                                          cnt, &visited, false, NULL, NULL);
          if (visited)
            BITMAP_FREE (visited);
        }
Index: gcc/tree-ssa-sccvn.c
===================================================================
--- gcc/tree-ssa-sccvn.c        (revision 273042)
+++ gcc/tree-ssa-sccvn.c        (working copy)
@@ -1648,9 +1648,16 @@ vn_reference_lookup_1 (vn_reference_t vr
 
 struct vn_walk_cb_data
 {
+  vn_walk_cb_data (vn_reference_t vr_, tree *last_vuse_ptr_,
+                   vn_lookup_kind vn_walk_kind_, bool tbaa_p_)
+    : vr (vr_), last_vuse_ptr (last_vuse_ptr_), vn_walk_kind (vn_walk_kind_),
+      tbaa_p (tbaa_p_)
+    {}
+
   vn_reference_t vr;
   tree *last_vuse_ptr;
   vn_lookup_kind vn_walk_kind;
+  bool tbaa_p;
 };
 
 /* Callback for walk_non_aliased_vuses.  Adjusts the vn_reference_t VR_
@@ -1927,6 +1934,33 @@ public:
 static rpo_elim *rpo_avail;
 basic_block vn_context_bb;
 
+/* Return true if BASE1 and BASE2 can be adjusted so they have the
+   same address and adjust *OFFSET1 and *OFFSET2 accordingly.
+   Otherwise return false.  */
+
+static bool
+adjust_offsets_for_equal_base_address (tree base1, poly_int64 *offset1,
+                                      tree base2, poly_int64 *offset2)
+{
+  poly_int64 soff;
+  if (TREE_CODE (base1) == MEM_REF
+      && TREE_CODE (base2) == MEM_REF)
+    {
+      if (mem_ref_offset (base1).to_shwi (&soff))
+       {
+         base1 = TREE_OPERAND (base1, 0);
+         *offset1 += soff * BITS_PER_UNIT;
+       }
+      if (mem_ref_offset (base2).to_shwi (&soff))
+       {
+         base2 = TREE_OPERAND (base2, 0);
+         *offset2 += soff * BITS_PER_UNIT;
+       }
+      return operand_equal_p (base1, base2, 0);
+    }
+  return operand_equal_p (base1, base2, OEP_ADDRESS_OF);
+}
+
 /* Callback for walk_non_aliased_vuses.  Tries to perform a lookup
    from the statement defining VUSE and if not successful tries to
    translate *REFP and VR_ through an aggregate copy at the definition
@@ -1966,7 +2000,7 @@ vn_reference_lookup_3 (ao_ref *ref, tree
                                                      get_alias_set (lhs),
                                                      TREE_TYPE (lhs), lhs_ops);
          if (lhs_ref_ok
-             && !refs_may_alias_p_1 (ref, &lhs_ref, true))
+             && !refs_may_alias_p_1 (ref, &lhs_ref, data->tbaa_p))
            {
              *disambiguate_only = true;
              return NULL;
@@ -2054,7 +2088,9 @@ vn_reference_lookup_3 (ao_ref *ref, tree
        }
     }
 
-  if (*disambiguate_only)
+  /* If we are looking for redundant stores do not create new hashtable
+     entries from aliasing defs with made up alias-sets.  */
+  if (*disambiguate_only || !data->tbaa_p)
     return (void *)-1;
 
   /* If we cannot constrain the size of the reference we cannot
@@ -2185,7 +2221,8 @@ vn_reference_lookup_3 (ao_ref *ref, tree
                                       &offset2, &size2, &maxsize2, &reverse);
       if (known_size_p (maxsize2)
          && known_eq (maxsize2, size2)
-         && operand_equal_p (base, base2, 0)
+         && adjust_offsets_for_equal_base_address (base, &offset,
+                                                   base2, &offset2)
          && known_subrange_p (offset, maxsize, offset2, size2))
        {
          tree val = build_zero_cst (vr->type);
@@ -2212,15 +2249,20 @@ vn_reference_lookup_3 (ao_ref *ref, tree
                   && is_gimple_min_invariant (SSA_VAL (gimple_assign_rhs1 
(def_stmt))))))
     {
       tree base2;
-      HOST_WIDE_INT offset2, size2;
+      poly_int64 offset2, size2, maxsize2;
+      HOST_WIDE_INT offset2i;
       bool reverse;
-      base2 = get_ref_base_and_extent_hwi (gimple_assign_lhs (def_stmt),
-                                          &offset2, &size2, &reverse);
+      base2 = get_ref_base_and_extent (gimple_assign_lhs (def_stmt),
+                                      &offset2, &size2, &maxsize2, &reverse);
       if (base2
          && !reverse
-         && size2 % BITS_PER_UNIT == 0
-         && offset2 % BITS_PER_UNIT == 0
-         && operand_equal_p (base, base2, 0)
+         && known_eq (maxsize2, size2)
+         && multiple_p (size2, BITS_PER_UNIT)
+         && multiple_p (offset2, BITS_PER_UNIT)
+         && adjust_offsets_for_equal_base_address (base, &offset,
+                                                   base2, &offset2)
+         && offset.is_constant (&offseti)
+         && offset2.is_constant (&offset2i)
          && known_subrange_p (offseti, maxsizei, offset2, size2))
        {
          /* We support up to 512-bit values (for V8DFmode).  */
@@ -2232,12 +2274,12 @@ vn_reference_lookup_3 (ao_ref *ref, tree
            rhs = SSA_VAL (rhs);
          len = native_encode_expr (rhs,
                                    buffer, sizeof (buffer),
-                                   (offseti - offset2) / BITS_PER_UNIT);
+                                   (offseti - offset2i) / BITS_PER_UNIT);
          if (len > 0 && len * BITS_PER_UNIT >= maxsizei)
            {
              tree type = vr->type;
              /* Make sure to interpret in a type that has a range
-                covering the whole access size.  */
+                covering the whole access size.  */
              if (INTEGRAL_TYPE_P (vr->type)
                  && maxsizei != TYPE_PRECISION (vr->type))
                type = build_nonstandard_integer_type (maxsizei,
@@ -2282,7 +2324,8 @@ vn_reference_lookup_3 (ao_ref *ref, tree
       if (!reverse
          && known_size_p (maxsize2)
          && known_eq (maxsize2, size2)
-         && operand_equal_p (base, base2, 0)
+         && adjust_offsets_for_equal_base_address (base, &offset,
+                                                   base2, &offset2)
          && known_subrange_p (offset, maxsize, offset2, size2)
          /* ???  We can't handle bitfield precision extracts without
             either using an alternate type for the BIT_FIELD_REF and
@@ -2652,10 +2695,10 @@ vn_reference_lookup_pieces (tree vuse, a
     {
       ao_ref r;
       unsigned limit = PARAM_VALUE (PARAM_SCCVN_MAX_ALIAS_QUERIES_PER_ACCESS);
-      vn_walk_cb_data data = { &vr1, NULL, kind };
+      vn_walk_cb_data data (&vr1, NULL, kind, true);
       if (ao_ref_init_from_vn_reference (&r, set, type, vr1.operands))
        *vnresult =
-         (vn_reference_t)walk_non_aliased_vuses (&r, vr1.vuse,
+         (vn_reference_t)walk_non_aliased_vuses (&r, vr1.vuse, true,
                                                  vn_reference_lookup_2,
                                                  vn_reference_lookup_3,
                                                  vuse_valueize, limit, &data);
@@ -2692,7 +2735,7 @@ vn_reference_lookup (tree op, tree vuse,
   vr1.operands = operands
     = valueize_shared_reference_ops_from_ref (op, &valuezied_anything);
   vr1.type = TREE_TYPE (op);
-  vr1.set = tbaa_p ? get_alias_set (op) : 0;
+  vr1.set = get_alias_set (op);
   vr1.hashcode = vn_reference_compute_hash (&vr1);
   if ((cst = fully_constant_vn_reference_p (&vr1)))
     return cst;
@@ -2709,11 +2752,9 @@ vn_reference_lookup (tree op, tree vuse,
          || !ao_ref_init_from_vn_reference (&r, vr1.set, vr1.type,
                                             vr1.operands))
        ao_ref_init (&r, op);
-      if (! tbaa_p)
-       r.ref_alias_set = r.base_alias_set = 0;
-      vn_walk_cb_data data = { &vr1, last_vuse_ptr, kind };
+      vn_walk_cb_data data (&vr1, last_vuse_ptr, kind, tbaa_p);
       wvnresult =
-       (vn_reference_t)walk_non_aliased_vuses (&r, vr1.vuse,
+       (vn_reference_t)walk_non_aliased_vuses (&r, vr1.vuse, tbaa_p,
                                                vn_reference_lookup_2,
                                                vn_reference_lookup_3,
                                                vuse_valueize, limit, &data);
Index: gcc/tree-ssa-scopedtables.c
===================================================================
--- gcc/tree-ssa-scopedtables.c (revision 273041)
+++ gcc/tree-ssa-scopedtables.c (working copy)
@@ -298,7 +298,7 @@ avail_exprs_stack::lookup_avail_expr (gi
            && TREE_CODE (gimple_assign_lhs (stmt)) == SSA_NAME
            && (ao_ref_init (&ref, gimple_assign_rhs1 (stmt)),
                ref.base_alias_set = ref.ref_alias_set = tbaa_p ? -1 : 0, true)
-           && walk_non_aliased_vuses (&ref, vuse2, vuse_eq, NULL, NULL,
+           && walk_non_aliased_vuses (&ref, vuse2, true, vuse_eq, NULL, NULL,
                                       limit, vuse1) != NULL))
        {
          if (insert)

Reply via email to