re PR tree-optimization/86844 (wrong code caused by store merging pass)
authorJakub Jelinek <jakub@redhat.com>
Wed, 12 Sep 2018 09:25:07 +0000 (11:25 +0200)
committerJakub Jelinek <jakub@gcc.gnu.org>
Wed, 12 Sep 2018 09:25:07 +0000 (11:25 +0200)
PR tree-optimization/86844
* gimple-ssa-store-merging.c
(imm_store_chain_info::coalesce_immediate): For overlapping stores, if
there are any overlapping stores in between them, make sure they are
also coalesced or we give up completely.

* gcc.c-torture/execute/pr86844.c: New test.
* gcc.dg/store_merging_22.c: New test.
* gcc.dg/store_merging_23.c: New test.

Co-Authored-By: Andreas Krebbel <krebbel@linux.ibm.com>
From-SVN: r264232

gcc/ChangeLog
gcc/gimple-ssa-store-merging.c
gcc/testsuite/ChangeLog
gcc/testsuite/gcc.c-torture/execute/pr86844.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/store_merging_22.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/store_merging_23.c [new file with mode: 0644]

index c08654dfe2bd7247695457e17d4154934394acd0..bd0d8668bd5d897bf8cb5fb52ebb0f560bc37caf 100644 (file)
@@ -1,3 +1,12 @@
+2018-09-12  Jakub Jelinek  <jakub@redhat.com>
+           Andreas Krebbel  <krebbel@linux.ibm.com>
+
+       PR tree-optimization/86844
+       * gimple-ssa-store-merging.c
+       (imm_store_chain_info::coalesce_immediate): For overlapping stores, if
+       there are any overlapping stores in between them, make sure they are
+       also coalesced or we give up completely.
+
 2018-09-12  Jakub Jelinek  <jakub@redhat.com>
 
        PR middle-end/87248
index 0ae45817347ff456393abd38f331cafca090e657..c8031e185704b4efa9a8046f8a6d2223c0563ddb 100644 (file)
@@ -2702,15 +2702,80 @@ imm_store_chain_info::coalesce_immediate_stores ()
        {
          /* Only allow overlapping stores of constants.  */
          if (info->rhs_code == INTEGER_CST
-             && merged_store->stores[0]->rhs_code == INTEGER_CST
-             && check_no_overlap (m_store_info, i, INTEGER_CST,
-                                  MAX (merged_store->last_order, info->order),
-                                  MAX (merged_store->start
-                                       + merged_store->width,
-                                       info->bitpos + info->bitsize)))
+             && merged_store->stores[0]->rhs_code == INTEGER_CST)
            {
-             merged_store->merge_overlapping (info);
-             goto done;
+             unsigned int last_order
+               = MAX (merged_store->last_order, info->order);
+             unsigned HOST_WIDE_INT end
+               = MAX (merged_store->start + merged_store->width,
+                      info->bitpos + info->bitsize);
+             if (check_no_overlap (m_store_info, i, INTEGER_CST,
+                                   last_order, end))
+               {
+                 /* check_no_overlap call above made sure there are no
+                    overlapping stores with non-INTEGER_CST rhs_code
+                    in between the first and last of the stores we've
+                    just merged.  If there are any INTEGER_CST rhs_code
+                    stores in between, we need to merge_overlapping them
+                    even if in the sort_by_bitpos order there are other
+                    overlapping stores in between.  Keep those stores as is.
+                    Example:
+                       MEM[(int *)p_28] = 0;
+                       MEM[(char *)p_28 + 3B] = 1;
+                       MEM[(char *)p_28 + 1B] = 2;
+                       MEM[(char *)p_28 + 2B] = MEM[(char *)p_28 + 6B];
+                    We can't merge the zero store with the store of two and
+                    not merge anything else, because the store of one is
+                    in the original order in between those two, but in
+                    store_by_bitpos order it comes after the last store that
+                    we can't merge with them.  We can merge the first 3 stores
+                    and keep the last store as is though.  */
+                 unsigned int len = m_store_info.length (), k = i;
+                 for (unsigned int j = i + 1; j < len; ++j)
+                   {
+                     store_immediate_info *info2 = m_store_info[j];
+                     if (info2->bitpos >= end)
+                       break;
+                     if (info2->order < last_order)
+                       {
+                         if (info2->rhs_code != INTEGER_CST)
+                           {
+                             /* Normally check_no_overlap makes sure this
+                                doesn't happen, but if end grows below, then
+                                we need to process more stores than
+                                check_no_overlap verified.  Example:
+                                   MEM[(int *)p_5] = 0;
+                                   MEM[(short *)p_5 + 3B] = 1;
+                                   MEM[(char *)p_5 + 4B] = _9;
+                                   MEM[(char *)p_5 + 2B] = 2;  */
+                             k = 0;
+                             break;
+                           }
+                         k = j;
+                         end = MAX (end, info2->bitpos + info2->bitsize);
+                       }
+                   }
+
+                 if (k != 0)
+                   {
+                     merged_store->merge_overlapping (info);
+
+                     for (unsigned int j = i + 1; j <= k; j++)
+                       {
+                         store_immediate_info *info2 = m_store_info[j];
+                         gcc_assert (info2->bitpos < end);
+                         if (info2->order < last_order)
+                           {
+                             gcc_assert (info2->rhs_code == INTEGER_CST);
+                             merged_store->merge_overlapping (info2);
+                           }
+                         /* Other stores are kept and not merged in any
+                            way.  */
+                       }
+                     ignore = k;
+                     goto done;
+                   }
+               }
            }
        }
       /* |---store 1---||---store 2---|
index c32dc77148629d6e0cc4e9beda58fb588482ba1a..1d38a8efb703a563d8077f56042699fd8e8cfec9 100644 (file)
@@ -1,3 +1,11 @@
+2018-09-12  Jakub Jelinek  <jakub@redhat.com>
+           Andreas Krebbel  <krebbel@linux.ibm.com>
+
+       PR tree-optimization/86844
+       * gcc.c-torture/execute/pr86844.c: New test.
+       * gcc.dg/store_merging_22.c: New test.
+       * gcc.dg/store_merging_23.c: New test.
+
 2018-09-12  Jakub Jelinek  <jakub@redhat.com>
 
        PR middle-end/87248
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr86844.c b/gcc/testsuite/gcc.c-torture/execute/pr86844.c
new file mode 100644 (file)
index 0000000..ead8922
--- /dev/null
@@ -0,0 +1,24 @@
+/* PR tree-optimization/86844 */
+
+__attribute__((noipa)) void
+foo (int *p)
+{
+  *p = 0;
+  *((char *)p + 3) = 1;
+  *((char *)p + 1) = 2;
+  *((char *)p + 2) = *((char *)p + 6);
+}
+
+int
+main ()
+{
+  int a[2] = { -1, 0 };
+  if (sizeof (int) != 4)
+    return 0;
+  ((char *)a)[6] = 3;
+  foo (a);
+  if (((char *)a)[0] != 0 || ((char *)a)[1] != 2
+      || ((char *)a)[2] != 3 || ((char *)a)[3] != 1)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/store_merging_22.c b/gcc/testsuite/gcc.dg/store_merging_22.c
new file mode 100644 (file)
index 0000000..bcd4235
--- /dev/null
@@ -0,0 +1,16 @@
+/* PR tree-optimization/86844 */
+/* { dg-do compile } */
+/* { dg-require-effective-target store_merge } */
+/* { dg-options "-O2 -fdump-tree-store-merging" } */
+
+void
+foo (int *p)
+{
+  *p = 0;
+  *((char *)p + 3) = 1;
+  *((char *)p + 1) = 2;
+  *((char *)p + 2) = *((char *)p + 38);
+}
+
+/* { dg-final { scan-tree-dump-times "Merging successful" 1 "store-merging" } } */
+/* { dg-final { scan-tree-dump-times "New sequence of 1 stores to replace old one of 3 stores" 1 "store-merging" } } */
diff --git a/gcc/testsuite/gcc.dg/store_merging_23.c b/gcc/testsuite/gcc.dg/store_merging_23.c
new file mode 100644 (file)
index 0000000..dd197fb
--- /dev/null
@@ -0,0 +1,16 @@
+/* PR tree-optimization/86844 */
+/* { dg-do compile } */
+/* { dg-require-effective-target store_merge } */
+/* { dg-options "-O2 -fdump-tree-store-merging" } */
+
+void
+foo (int *p)
+{
+  *p = 0;
+  int one = 1;
+  __builtin_memcpy ((char *) p + 3, &one, sizeof (int));
+  *((char *)p + 4) = *((char *)p + 36);
+  *((char *)p + 1) = 2;
+}
+
+/* { dg-final { scan-tree-dump-not "Merging successful" "store-merging" } } */