public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc(refs/vendors/redhat/heads/gcc-8-branch)] combine: Fix find_split_point handling of constant store into ZERO_EXTRACT [PR93908]
@ 2020-09-17 16:46 Jakub Jelinek
  0 siblings, 0 replies; only message in thread
From: Jakub Jelinek @ 2020-09-17 16:46 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:dbfe8775e356dd83b2feced7dd8e94f10bde7f76

commit dbfe8775e356dd83b2feced7dd8e94f10bde7f76
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Feb 25 13:56:47 2020 +0100

    combine: Fix find_split_point handling of constant store into ZERO_EXTRACT [PR93908]
    
    git is miscompiled on s390x-linux with -O2 -march=zEC12 -mtune=z13.
    I've managed to reduce it into the following testcase.  The problem is that
    during combine we see the s->k = -1; bitfield store and change the SET_SRC
    from a pseudo into a constant:
    (set (zero_extract:DI (mem/j:HI (plus:DI (reg/v/f:DI 60 [ s ])
                    (const_int 10 [0xa])) [0 +0 S2 A16])
            (const_int 2 [0x2])
            (const_int 7 [0x7]))
        (const_int -1 [0xffffffffffffffff]))
    This on s390x with the above option isn't recognized as valid instruction,
    so find_split_point decides to handle it as IOR or IOR/AND.
    src is -1, mask is 3 and pos is 7.
    src != mask (this is also incorrect, we want to set all (both) bits in the
    bitfield), so we go for IOR/AND, but instead of trying
    mem = (mem & ~0x180) | ((-1 << 7) & 0x180)
    we actually try
    mem = (mem & ~0x180) | (-1 << 7)
    and that is further simplified into:
    mem = mem | (-1 << 7)
    aka
    mem = mem | 0xff80
    which doesn't set just the 2-bit bitfield, but also many other bitfields
    that shouldn't be touched.
    We really should do:
    mem = mem | 0x180
    instead.
    The problem is that we assume that no bits but those low len (2 here) will
    be set in the SET_SRC, but there is nothing that can prevent that, we just
    should ignore the other bits.
    
    The following patch fixes it by masking src with mask, this way already
    the src == mask test will DTRT, and as the code for or_mask uses
    gen_int_mode, if the most significant bit is set after shifting it left by
    pos, it will be properly sign-extended.
    
    2020-02-25  Jakub Jelinek  <jakub@redhat.com>
    
            PR rtl-optimization/93908
            * combine.c (find_split_point): For store into ZERO_EXTRACT, and src
            with mask.
    
            * gcc.c-torture/execute/pr93908.c: New test.

Diff:
---
 gcc/ChangeLog                                 |  6 +++
 gcc/combine.c                                 |  5 +--
 gcc/testsuite/ChangeLog                       |  5 +++
 gcc/testsuite/gcc.c-torture/execute/pr93908.c | 54 +++++++++++++++++++++++++++
 gcc/testsuite/gcc.target/s390/pr93908.c       |  5 +++
 5 files changed, 72 insertions(+), 3 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 700f520f97d..3cbf6b6a49c 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,9 @@
+2020-02-25  Jakub Jelinek  <jakub@redhat.com>
+
+	PR rtl-optimization/93908
+	* combine.c (find_split_point): For store into ZERO_EXTRACT, and src
+	with mask.
+
 2019-02-25  Eric Botcazou  <ebotcazou@adacore.com>
 
 	* dwarf2out.c (dwarf2out_size_function): Run in early-DWARF mode.
diff --git a/gcc/combine.c b/gcc/combine.c
index 6a9638d982c..1ba66c154a5 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -5037,10 +5037,9 @@ find_split_point (rtx *loc, rtx_insn *insn, bool set_src)
 	{
 	  HOST_WIDE_INT pos = INTVAL (XEXP (SET_DEST (x), 2));
 	  unsigned HOST_WIDE_INT len = INTVAL (XEXP (SET_DEST (x), 1));
-	  unsigned HOST_WIDE_INT src = INTVAL (SET_SRC (x));
 	  rtx dest = XEXP (SET_DEST (x), 0);
-	  unsigned HOST_WIDE_INT mask
-	    = (HOST_WIDE_INT_1U << len) - 1;
+	  unsigned HOST_WIDE_INT mask = (HOST_WIDE_INT_1U << len) - 1;
+	  unsigned HOST_WIDE_INT src = INTVAL (SET_SRC (x)) & mask;
 	  rtx or_mask;
 
 	  if (BITS_BIG_ENDIAN)
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index c669c736129..beb1092cd87 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-02-25  Jakub Jelinek  <jakub@redhat.com>
+
+	PR rtl-optimization/93908
+	* gcc.c-torture/execute/pr93908.c: New test.
+
 2019-02-25  Eric Botcazou  <ebotcazou@adacore.com>
 
 	* gnat.dg/lto24.adb: New test.
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr93908.c b/gcc/testsuite/gcc.c-torture/execute/pr93908.c
new file mode 100644
index 00000000000..cb6e3dda2ea
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr93908.c
@@ -0,0 +1,54 @@
+/* PR rtl-optimization/93908 */
+
+struct T
+{
+  int b;
+  int c;
+  unsigned short d;
+  unsigned e:1, f:1, g:1, h:2, i:1, j:1;
+  signed int k:2;
+};
+
+struct S
+{
+  struct T s;
+  char c[64];
+} buf[2];
+
+__attribute__ ((noipa)) void *
+baz (void)
+{
+  static int cnt;
+  return (void *) &buf[cnt++];
+}
+
+static inline __attribute__ ((always_inline)) struct T *
+bar (const char *a)
+{
+  struct T *s;
+  s = baz ();
+  s->b = 1;
+  s->k = -1;
+  return s;
+}
+
+__attribute__ ((noipa)) void
+foo (const char *x, struct T **y)
+{
+  struct T *l = bar (x);
+  struct T *m = bar (x);
+  y[0] = l;
+  y[1] = m;
+}
+
+int
+main ()
+{
+  struct T *r[2];
+  foo ("foo", r);
+  if (r[0]->e || r[0]->f || r[0]->g || r[0]->h || r[0]->i || r[0]->j || r[0]->k != -1)
+    __builtin_abort ();
+  if (r[1]->e || r[1]->f || r[1]->g || r[1]->h || r[1]->i || r[1]->j || r[1]->k != -1)
+    __builtin_abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/s390/pr93908.c b/gcc/testsuite/gcc.target/s390/pr93908.c
new file mode 100644
index 00000000000..e5949050a08
--- /dev/null
+++ b/gcc/testsuite/gcc.target/s390/pr93908.c
@@ -0,0 +1,5 @@
+/* PR rtl-optimization/93908 */
+/* { dg-do run { target s390_zEC12_hw } } */
+/* { dg-options "-O2 -march=zEC12 -mtune=z13" } */
+
+#include "../../gcc.c-torture/execute/pr93908.c"


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2020-09-17 16:46 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-17 16:46 [gcc(refs/vendors/redhat/heads/gcc-8-branch)] combine: Fix find_split_point handling of constant store into ZERO_EXTRACT [PR93908] Jakub Jelinek

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).