diff mbox series

Make asm not contain prefixed addresses.

Message ID 20210202042442.GA24643@ibm-toto.the-meissners.org
State New
Headers show
Series Make asm not contain prefixed addresses. | expand

Commit Message

Michael Meissner Feb. 2, 2021, 4:24 a.m. UTC 
From 4ceff15935a16da9ec5833279807855a8afc47cd Mon Sep 17 00:00:00 2001
From: Michael Meissner <meissner@linux.ibm.com>
Date: Mon, 1 Feb 2021 22:19:57 -0500
Subject: [PATCH] Make asm not contain prefixed addresses.

In PR target/98519, the assembler does not like asm memory references that are
prefixed.  We can't automatically change the instruction to prefixed form with
a 'p' like we do for normal RTL insns, since this is assembly code.  Instead,
the patch prevents prefixed memory addresses from being passed by default.

This patch uses the TARGET_MD_ASM_ADJUST target hook to change the 'm' and 'o'
constraints to be 'em' and 'eo'.  The 'em' and 'eo' constraints do not allow
prefixed instructions.  In addition, a new constraint 'ep' is added to
explicitly require a prefixed instruction.

I have tested this on a little endian power9 system in doing a bootstrap build
and make check.  There were no regressions.  Can I check this into the master
branch?

I would like to also backport this change to the GCC 10 branch, since the bug
can happen in that release as well.

[gcc]
2021-02-01  Michael Meissner  <meissner@linux.ibm.com>

	PR target/98519
	* config/rs6000/constraints.md (em): New constraint.
	(eo): New constraint.
	(ep): New constraint.
	* config/rs6000/rs6000.md (rs6000_md_asm_adjust): If prefixed
	addresses are allowed, change the 'm' and 'o' constraints to 'em'
	and 'eo' that do not allow prefixed instructions.
	* doc/md.texi (PowerPC and IBM RS6000 constraints): Document 'em',
	'eo', and 'ep' constraints.

[gcc/testsuite]
2021-02-01  Michael Meissner  <meissner@linux.ibm.com>

	PR target/98519
	* gcc.target/powerpc/pr98519.c: New test.
---
 gcc/config/rs6000/constraints.md           | 19 ++++++++
 gcc/config/rs6000/rs6000.c                 | 56 +++++++++++++++++++++-
 gcc/doc/md.texi                            |  9 ++++
 gcc/testsuite/gcc.target/powerpc/pr98519.c | 17 +++++++
 4 files changed, 100 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/powerpc/pr98519.c

Comments

Segher Boessenkool Feb. 2, 2021, 9:57 p.m. UTC | #1 
Hi,

On Mon, Feb 01, 2021 at 11:24:42PM -0500, Michael Meissner wrote:
> In PR target/98519, the assembler does not like asm memory references that are
> prefixed.  We can't automatically change the instruction to prefixed form with
> a 'p' like we do for normal RTL insns, since this is assembly code.

It is incorrect in general, too, for at least three reasons.

> Instead,
> the patch prevents prefixed memory addresses from being passed by default.
> 
> This patch uses the TARGET_MD_ASM_ADJUST target hook to change the 'm' and 'o'
> constraints to be 'em' and 'eo'.

This does not work correctly, and is a gross misuse of this hook anyway.

Like I said before, just make 'm' (or even general_operand) not allow
prefixed memory in inline asm.

> +	  while ((ch = *constraint++) != '\0')

Don't do assignments (or any other surprising side effects) in
conditionals please.  Code should be *readable*, not a puzzle.

0 is written as 0, not as 000 or '\0' or 0x0 or anything else.

> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr98519.c
> @@ -0,0 +1,17 @@
> +/* { dg-do compile } */

This is the default.

> +/* { dg-require-effective-target powerpc_prefixed_addr } */

You do not want this line, you want to compile this *always*.  You set
cpu=power10 anyway!  Also, the test should work *without it*!

> +/* { dg-final { scan-assembler-not {\m[@]pcrel\M} } } */

You can just write @ instead of [@].  Putting \m immediately in front of
a non-letter (or \M immediately after) does not do anything, either.


Segher
diff mbox series

Patch

diff --git a/gcc/config/rs6000/constraints.md b/gcc/config/rs6000/constraints.md
index 561ce9797af..56a7eda4c85 100644
--- a/gcc/config/rs6000/constraints.md
+++ b/gcc/config/rs6000/constraints.md
@@ -245,6 +245,25 @@  (define_memory_constraint "es"
   (and (match_code "mem")
        (match_test "GET_RTX_CLASS (GET_CODE (XEXP (op, 0))) != RTX_AUTOINC")))
 
+;; Non-prefixed memory
+(define_memory_constraint "em"
+  "@internal Memory operand that is not prefixed."
+  (and (match_code "mem")
+       (not (match_operand 0 "prefixed_memory"))))
+
+;; Non-prefixed memory that is also offsettable
+(define_memory_constraint "eo"
+  "@internal Memory operand that is not prefixed but is offsettable."
+  (and (match_code "mem")
+       (not (match_operand 0 "prefixed_memory"))
+       (match_operand 0 "offsettable_mem_operand")))
+
+;; prefixed memory
+(define_memory_constraint "ep"
+  "@internal Memory operand that is prefixed."
+  (and (match_code "mem")
+       (match_operand 0 "prefixed_memory")))
+
 (define_memory_constraint "Q"
   "A memory operand addressed by just a base register."
   (and (match_code "mem")
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index ec068c58aa5..726928a4d32 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -3414,9 +3414,63 @@  rs6000_builtin_mask_calculate (void)
 
 static rtx_insn *
 rs6000_md_asm_adjust (vec<rtx> &/*outputs*/, vec<rtx> &/*inputs*/,
-		      vec<const char *> &/*constraints*/,
+		      vec<const char *> &constraints,
 		      vec<rtx> &clobbers, HARD_REG_SET &clobbered_regs)
 {
+  /* If prefixed addresses are allowed, change the "m" constraint to "em" and
+     the "o" constraint to "eo".  The "em" and "eo" constraints do not allow
+     prefixed memory.  */
+  if (TARGET_PREFIXED)
+    {
+      size_t max_len = 0;
+      for (unsigned i = 0; i < constraints.length (); ++i)
+	{
+	  size_t len = strlen (constraints[i]);
+	  if (len > max_len)
+	    max_len = len;
+	}
+
+      char *buffer = (char *) alloca (2 * max_len + 1);
+      for (unsigned i = 0; i < constraints.length (); ++i)
+	{
+	  const char *constraint = constraints[i];
+	  char *new_constraint = buffer;
+	  char ch;
+	  bool found_m_or_o = false;
+
+	  while ((ch = *constraint++) != '\0')
+	    switch (ch)
+	      {
+	      default:
+		*new_constraint++ = ch;
+		break;
+
+		/* If we found 'm' or 'o', convert it to 'em' or 'eo' so that
+		   prefixed memory is not allowed.  */
+	      case 'm':
+	      case 'o':
+		found_m_or_o = true;
+		*new_constraint++ = 'e';
+		*new_constraint++ = ch;
+		break;
+
+		/* 'e' and 'w' begin two letter constraints.  */
+	      case 'e':
+	      case 'w':
+		*new_constraint++ = ch;
+		*new_constraint++ = *constraint++;
+		break;
+	      }
+
+	  /* If we had 'm' or 'o', update the constraints.  */
+	  if (found_m_or_o)
+	    {
+	      *new_constraint = '\0';
+	      constraints[i] = xstrdup (buffer);
+	    }
+	}
+    }
+
   clobbers.safe_push (gen_rtx_REG (SImode, CA_REGNO));
   SET_HARD_REG_BIT (clobbered_regs, CA_REGNO);
   return NULL;
diff --git a/gcc/doc/md.texi b/gcc/doc/md.texi
index e3686dbfe61..c0587afd78e 100644
--- a/gcc/doc/md.texi
+++ b/gcc/doc/md.texi
@@ -3369,6 +3369,15 @@  are now only allowed when @code{<} or @code{>} is used, @code{es} is
 basically the same as @code{m} without @code{<} and @code{>}.
 @end ifset
 
+@item em
+A memory operand that is not prefixed.
+
+@item eo
+A memory operand that is offsettable but it is not prefixed.
+
+@item ep
+A memory operand that is prefixed.
+
 @item Q
 A memory operand addressed by just a base register.
 
diff --git a/gcc/testsuite/gcc.target/powerpc/pr98519.c b/gcc/testsuite/gcc.target/powerpc/pr98519.c
new file mode 100644
index 00000000000..c333bf42653
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr98519.c
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-require-effective-target powerpc_prefixed_addr } */
+/* { dg-options "-O2 -mdejagnu-cpu=power10" } */
+
+typedef vector double vf64_t;
+
+static double test_f64[16];
+
+vf64_t
+bug (void)
+{
+  vf64_t j0;
+  __asm__("lxsd%X1 %0,%1;" : "=v" (j0) : "m" (test_f64));
+  return j0;
+}
+
+/* { dg-final { scan-assembler-not {\m[@]pcrel\M} } } */