[RS6000] lqarx and stqcx. registers

[RS6000] lqarx and stqcx. registers

[Alan Modra]

lqarx RT and stqcx. RS are valid only with even numbered gprs.  The
predicate to enforce this happens to allow a loophole, closed by this
patch.

This pattern created by combine:
Trying 8 -> 9:
Successfully matched this instruction:
(set (subreg:PTI (reg:TI 155 [ D.2357 ]) 0)
    (unspec_volatile:PTI [
            (mem/v:TI (reg/v/f:DI 157 [ mptr ]) [-1  S16 A128])
        ] UNSPECV_LL))

is seen by reload as needing to reload pseudo 155 in TI mode, which
has no requirement that the reg be even.  Apparently, nothing checks
the predicate again after reload.

We only see this problem on gcc-5 and gcc-4.9, because on gcc-6 we
don't define WORD_REGISTER_OPERATIONS and combine happens to have a
bug in simplify_set that prevents it creating the problem subregs.
See https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02377.html

Bootstrapped and regression tested powerpc64-linux biarch on master
both with and without the combine bug, and on gcc-5.  OK for master
and active branches?

gcc/
	PR target/69548
	* config/rs6000/predicates.md (quad_int_reg_operand): Don't
	allow subregs.
gcc/testsuite/
	* gcc.target/powerpc/pr69548.c: New test.

diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index b8f14fd..302303c 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -375,20 +375,19 @@
 
 ;; Return 1 if op is a general purpose register that is an even register
 ;; which suitable for a load/store quad operation
+;; Subregs are not allowed here because when they are combine can
+;; create (subreg:PTI (reg:TI pseudo)) which will cause reload to
+;; think the innermost reg needs reloading, in TImode instead of
+;; PTImode.  So reload will choose a reg in TImode which has no
+;; requirement that the reg be even.
 (define_predicate "quad_int_reg_operand"
-  (match_operand 0 "register_operand")
+  (match_code "reg")
 {
   HOST_WIDE_INT r;
 
   if (!TARGET_QUAD_MEMORY && !TARGET_QUAD_MEMORY_ATOMIC)
     return 0;
 
-  if (GET_CODE (op) == SUBREG)
-    op = SUBREG_REG (op);
-
-  if (!REG_P (op))
-    return 0;
-
   r = REGNO (op);
   if (r >= FIRST_PSEUDO_REGISTER)
     return 1;
diff --git a/gcc/testsuite/gcc.target/powerpc/pr69548.c b/gcc/testsuite/gcc.target/powerpc/pr69548.c
new file mode 100644
index 0000000..439f588
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr69548.c
@@ -0,0 +1,11 @@
+/* { dg-do assemble { target { powerpc*-*-* && lp64 } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-skip-if "do not override -mcpu" { powerpc*-*-* } { "-mcpu=*" } { "-mcpu=power8" } } */
+/* { dg-options "-mcpu=power8 -Os -mbig" } */
+
+__int128
+quad_exchange (__int128 *ptr, __int128 newval)
+{
+  return __atomic_exchange_n (ptr, newval, __ATOMIC_RELAXED);
+}

-- 
Alan Modra
Australia Development Lab, IBM

Re: [RS6000] lqarx and stqcx. registers

[David Edelsohn]

On Sun, Jan 31, 2016 at 5:28 PM, Alan Modra <amodra@gmail.com> wrote:
> lqarx RT and stqcx. RS are valid only with even numbered gprs.  The
> predicate to enforce this happens to allow a loophole, closed by this
> patch.
>
> This pattern created by combine:
> Trying 8 -> 9:
> Successfully matched this instruction:
> (set (subreg:PTI (reg:TI 155 [ D.2357 ]) 0)
>     (unspec_volatile:PTI [
>             (mem/v:TI (reg/v/f:DI 157 [ mptr ]) [-1  S16 A128])
>         ] UNSPECV_LL))
>
> is seen by reload as needing to reload pseudo 155 in TI mode, which
> has no requirement that the reg be even.  Apparently, nothing checks
> the predicate again after reload.
>
> We only see this problem on gcc-5 and gcc-4.9, because on gcc-6 we
> don't define WORD_REGISTER_OPERATIONS and combine happens to have a
> bug in simplify_set that prevents it creating the problem subregs.
> See https://gcc.gnu.org/ml/gcc-patches/2016-01/msg02377.html
>
> Bootstrapped and regression tested powerpc64-linux biarch on master
> both with and without the combine bug, and on gcc-5.  OK for master
> and active branches?
>
> gcc/
>         PR target/69548
>         * config/rs6000/predicates.md (quad_int_reg_operand): Don't
>         allow subregs.
> gcc/testsuite/
>         * gcc.target/powerpc/pr69548.c: New test.

Okay.

Thanks, David