public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Bill Schmidt <wschmidt@linux.ibm.com>
To: GCC Patches <gcc-patches@gcc.gnu.org>
Cc: Segher Boessenkool <segher@kernel.crashing.org>,
	David Edelsohn <dje.gcc@gmail.com>
Subject: [PATCH, committed] rs6000: Don't let swaps pass break multiply low-part (PR101129)
Date: Thu, 15 Jul 2021 10:25:20 -0500	[thread overview]
Message-ID: <b4b7c76b-1374-d417-e503-425c4538630f@linux.ibm.com> (raw)

Hi,

Segher preapproved this patch in https://gcc.gnu.org/PR101129. It 
differs slightly from what was posted there, needing an additional test 
to ensure the insn is a SET.  The patch also includes the test case 
provided by the OP.  Bootstrap and regtest succeeded on P9 little-endian.

This bug has been around a long time, so the fix should be backported to 
all open releases.  Is this okay after some burn-in time?

Thanks!
Bill

rs6000: Don't let swaps pass break multiply low-part (PR101129)

2021-07-15  Bill Schmidt  <wschmidt@linux.ibm.com>

gcc/
	* config/rs6000/rs6000-p8swap.c (has_part_mult): New.
	(rs6000_analyze_swaps): Insns containing a subreg of a mult are
	not swappable.

gcc/testsuite/
	* gcc.target/powerpc/pr101129.c: New.

diff --git a/gcc/config/rs6000/rs6000-p8swap.c b/gcc/config/rs6000/rs6000-p8swap.c
index 21cbcb2e28a..6b559aa5061 100644
--- a/gcc/config/rs6000/rs6000-p8swap.c
+++ b/gcc/config/rs6000/rs6000-p8swap.c
@@ -1523,6 +1523,22 @@ replace_swap_with_copy (swap_web_entry *insn_entry, unsigned i)
    insn->set_deleted ();
  }
  
+/* INSN is known to contain a SUBREG, which we can normally handle,
+   but if the SUBREG itself contains a MULT then we need to leave it alone
+   to avoid turning a mult_hipart into a mult_lopart, for example.  */
+static bool
+has_part_mult (rtx_insn *insn)
+{
+  rtx body = PATTERN (insn);
+  if (GET_CODE (body) != SET)
+    return false;
+  rtx src = SET_SRC (body);
+  if (GET_CODE (src) != SUBREG)
+    return false;
+  rtx inner = XEXP (src, 0);
+  return (GET_CODE (inner) == MULT);
+}
+
  /* Make NEW_MEM_EXP's attributes and flags resemble those of
     ORIGINAL_MEM_EXP.  */
  static void
@@ -2501,6 +2517,9 @@ rs6000_analyze_swaps (function *fun)
  		    insn_entry[uid].is_swappable = 0;
  		  else if (special != SH_NONE)
  		    insn_entry[uid].special_handling = special;
+		  else if (insn_entry[uid].contains_subreg
+			   && has_part_mult (insn))
+		    insn_entry[uid].is_swappable = 0;
  		  else if (insn_entry[uid].contains_subreg)
  		    insn_entry[uid].special_handling = SH_SUBREG;
  		}
diff --git a/gcc/testsuite/gcc.target/powerpc/pr101129.c b/gcc/testsuite/gcc.target/powerpc/pr101129.c
new file mode 100644
index 00000000000..1abc12480e4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr101129.c
@@ -0,0 +1,35 @@
+/* { dg-do run } */
+/* { dg-require-effective-target p8vector_hw } */
+/* { dg-options "-mdejagnu-cpu=power8 -O " } */
+
+/* PR101129: The swaps pass was turning a mult-lopart into a mult-hipart.
+   Make sure we aren't doing that anymore.  */
+
+typedef unsigned char u8;
+typedef unsigned char __attribute__((__vector_size__ (8))) U;
+typedef unsigned char __attribute__((__vector_size__ (16))) V;
+typedef unsigned int u32;
+typedef unsigned long long u64;
+typedef __int128 u128;
+
+u8 g;
+U u;
+
+void
+foo0 (u32 u32_0, U *ret)
+{
+  u128 u128_2 = u32_0 * (u128)((V){ 5 } > (u32_0 & 4));
+  u64 u64_r = u128_2 >> 64;
+  u8 u8_r = u64_r + g;
+  *ret = u + u8_r;
+}
+
+int
+main (void)
+{
+  U x;
+  foo0 (7, &x);
+  for (unsigned i = 0; i < sizeof (x); i++)
+    if (x[i] != 0) __builtin_abort();
+  return 0;
+}


             reply	other threads:[~2021-07-15 15:25 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-15 15:25 Bill Schmidt [this message]
2021-07-15 15:29 ` David Edelsohn

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b4b7c76b-1374-d417-e503-425c4538630f@linux.ibm.com \
    --to=wschmidt@linux.ibm.com \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=segher@kernel.crashing.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).