diff mbox

[powerpc] , PR 57744, fix quad word atomic instructions to use even/odd registers

Message ID 20130628150913.GA25849@ibm-tiger.the-meissners.org
State New
Headers show

Commit Message

Michael Meissner June 28, 2013, 3:09 p.m. UTC 
This patch fixes PR 57744, where the compiler did not allocate even/odd
register combination for the lqarx and stqcx. instructions.  I traced the
problem down to the fact that PTImode can only go in even GPRs (in part to
support the quad word instructions), while TImode can go in even/odd GPRs as
well as the VSX regsiters.  In the example that showed the problem, the TImode
value is passed to the function in an odd/even register combination.  By
setting MODES_TIEABLE_P, the compiler no longer assumes that TImode and PTImode
automatically share registers.  While I was modifying MODES_TIEABLE_P, I also
made similar changes for TDmode (which needs even FPR registers), and small
integers (which can't go in floating point/VSX registers).

This patch bootstraps fine, and has no regressions.  Is it ok to apply this
patch?

[gcc]
2013-06-28  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/57744
	* config/rs6000/rs6000.h (SMALL_INT_MODE): New macro, to recognize
	small integer modes.
	(MODES_TIEABLE_P): Do not allow PTImode or TDmode to tie with any
	other modes.  Don't allow small integer modes to tie with modes
	that can go in floating point/VSX registers.  Eliminate Altivec
	vector mode tests, since these are a subset of ALTIVEC or VSX
	vector modes.  Simplify code, to return 0 if testing MODE2 for a
	condition, if we've already tested MODE1 for the same condition.

[gcc/testsuite]
2013-06-28  Michael Meissner  <meissner@linux.vnet.ibm.com>

	PR target/57744
	* gcc.target/powerpc/pr57744.c: New test to make sure lqarx and
	stqcx. get even registers.

Comments

David Edelsohn June 28, 2013, 3:39 p.m. UTC | #1 
On Fri, Jun 28, 2013 at 11:09 AM, Michael Meissner
<meissner@linux.vnet.ibm.com> wrote:
> This patch fixes PR 57744, where the compiler did not allocate even/odd
> register combination for the lqarx and stqcx. instructions.  I traced the
> problem down to the fact that PTImode can only go in even GPRs (in part to
> support the quad word instructions), while TImode can go in even/odd GPRs as
> well as the VSX regsiters.  In the example that showed the problem, the TImode
> value is passed to the function in an odd/even register combination.  By
> setting MODES_TIEABLE_P, the compiler no longer assumes that TImode and PTImode
> automatically share registers.  While I was modifying MODES_TIEABLE_P, I also
> made similar changes for TDmode (which needs even FPR registers), and small
> integers (which can't go in floating point/VSX registers).
>
> This patch bootstraps fine, and has no regressions.  Is it ok to apply this
> patch?
>
> [gcc]
> 2013-06-28  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>         PR target/57744
>         * config/rs6000/rs6000.h (SMALL_INT_MODE): New macro, to recognize
>         small integer modes.
>         (MODES_TIEABLE_P): Do not allow PTImode or TDmode to tie with any
>         other modes.  Don't allow small integer modes to tie with modes
>         that can go in floating point/VSX registers.  Eliminate Altivec
>         vector mode tests, since these are a subset of ALTIVEC or VSX
>         vector modes.  Simplify code, to return 0 if testing MODE2 for a
>         condition, if we've already tested MODE1 for the same condition.
>
> [gcc/testsuite]
> 2013-06-28  Michael Meissner  <meissner@linux.vnet.ibm.com>
>
>         PR target/57744
>         * gcc.target/powerpc/pr57744.c: New test to make sure lqarx and
>         stqcx. get even registers.

All integer modes should be tieable. If normal, scalar modes are not
tieable, we have more severe problems.

The SMALL_INT_MODE macro should have had a comment in rs6000.h, not in
the ChangeLog. But the SMALL_INT_MODE change to MODES_TIEABLE_P should
not be included.

Only the PTI change is okay.

- David
diff mbox

Patch

Index: gcc/config/rs6000/rs6000.h
===================================================================
--- gcc/config/rs6000/rs6000.h	(revision 200470)
+++ gcc/config/rs6000/rs6000.h	(working copy)
@@ -1172,6 +1172,11 @@  enum data_align { align_abi, align_opt, 
 #define PAIRED_VECTOR_MODE(MODE)        \
          ((MODE) == V2SFmode)            
 
+#define SMALL_INT_MODE(MODE)		\
+	((MODE) == QImode		\
+	 || (MODE) == HImode		\
+	 || (MODE) == SImode)
+
 /* Value is TRUE if hard register REGNO can hold a value of
    machine-mode MODE.  */
 #define HARD_REGNO_MODE_OK(REGNO, MODE) \
@@ -1180,28 +1185,40 @@  enum data_align { align_abi, align_opt, 
 /* Value is 1 if it is a good idea to tie two pseudo registers
    when one has mode MODE1 and one has mode MODE2.
    If HARD_REGNO_MODE_OK could produce different values for MODE1 and MODE2,
-   for any hard reg, then this must be 0 for correct output.  */
+   for any hard reg, then this must be 0 for correct output.
+
+   PTImode and TDmode cannot tie with other modes because they are restricted
+   to even registers, and it would create problems if an odd register was tried
+   to tie with PTI/TDmode.  Similarly, restrict small integer modes
+   (QI/HI/SImode) from being tied with DImode/TImode/etc. since they can't go in
+   floating point or VSX registers.  */
 #define MODES_TIEABLE_P(MODE1, MODE2)		\
-  (SCALAR_FLOAT_MODE_P (MODE1)			\
+  ((MODE1) == (MODE2)				\
+   ? 1						\
+   : ((MODE1) == PTImode || (MODE2) == PTImode) \
+   ? 0						\
+   : ((MODE1) == TDmode || (MODE2) == TDmode)	\
+   ? 0						\
+   : SMALL_INT_MODE (MODE1)			\
+   ? SMALL_INT_MODE (MODE2)			\
+   : SMALL_INT_MODE (MODE2)			\
+   ? 0						\
+   : SCALAR_FLOAT_MODE_P (MODE1)		\
    ? SCALAR_FLOAT_MODE_P (MODE2)		\
    : SCALAR_FLOAT_MODE_P (MODE2)		\
-   ? SCALAR_FLOAT_MODE_P (MODE1)		\
+   ? 0						\
    : GET_MODE_CLASS (MODE1) == MODE_CC		\
    ? GET_MODE_CLASS (MODE2) == MODE_CC		\
    : GET_MODE_CLASS (MODE2) == MODE_CC		\
-   ? GET_MODE_CLASS (MODE1) == MODE_CC		\
+   ? 0						\
    : SPE_VECTOR_MODE (MODE1)			\
    ? SPE_VECTOR_MODE (MODE2)			\
    : SPE_VECTOR_MODE (MODE2)			\
-   ? SPE_VECTOR_MODE (MODE1)			\
+   ? 0						\
    : ALTIVEC_OR_VSX_VECTOR_MODE (MODE1)		\
    ? ALTIVEC_OR_VSX_VECTOR_MODE (MODE2)		\
    : ALTIVEC_OR_VSX_VECTOR_MODE (MODE2)		\
-   ? ALTIVEC_OR_VSX_VECTOR_MODE (MODE1)		\
-   : ALTIVEC_VECTOR_MODE (MODE1)		\
-   ? ALTIVEC_VECTOR_MODE (MODE2)		\
-   : ALTIVEC_VECTOR_MODE (MODE2)		\
-   ? ALTIVEC_VECTOR_MODE (MODE1)		\
+   ? 0						\
    : 1)
 
 /* Post-reload, we can't use any new AltiVec registers, as we already
Index: gcc/testsuite/gcc.target/powerpc/pr57744.c
===================================================================
--- gcc/testsuite/gcc.target/powerpc/pr57744.c	(revision 0)
+++ gcc/testsuite/gcc.target/powerpc/pr57744.c	(revision 0)
@@ -0,0 +1,37 @@ 
+/* { dg-do run { target { powerpc*-*-* && lp64 } } } */
+/* { dg-skip-if "" { powerpc*-*-darwin* } { "*" } { "" } } */
+/* { dg-require-effective-target powerpc_p8vector_ok } */
+/* { dg-options "-mcpu=power8 -O3" } */
+
+typedef unsigned U_16 __attribute__((mode(TI)));
+
+extern int libat_compare_exchange_16 (U_16 *, U_16 *, U_16, int, int)
+  __attribute__((__noinline__));
+
+/* PR 57744: lqarx/stqcx needs even/odd register pairs.  The assembler will
+   complain if the compiler gets an odd/even register pair.  Create a function
+   which has the 16 byte compare and exchange instructions, but don't actually
+   execute it, so that we can detect these failures on older machines. */
+
+int
+libat_compare_exchange_16 (U_16 *mptr, U_16 *eptr, U_16 newval,
+         int smodel, int fmodel __attribute__((unused)))
+{
+  if (((smodel) == 0))
+    return __atomic_compare_exchange_n (mptr, eptr, newval, 0, 0, 0);
+  else if (((smodel) != 5))
+    return __atomic_compare_exchange_n (mptr, eptr, newval, 0, 4, 0);
+  else
+    return __atomic_compare_exchange_n (mptr, eptr, newval, 0, 5, 0);
+}
+
+U_16 a = 1, b = 1, c = -2;
+volatile int do_test = 0;
+
+int main (void)
+{
+  if (do_test && !libat_compare_exchange_16 (&a, &b, c, 0, 0))
+    aborrt ();
+
+  return 0;
+}