[PATCH, i386]: Improve LIMIT_RELOAD_CLASSES

[Uros Bizjak]

On Sat, Aug 4, 2012 at 2:26 PM, Uros Bizjak <ubizjak@gmail.com> wrote:

>>> Without this, on the new testcase we hit the assert in
>>> inline_secondary_memory_needed. The comment before the function states:
>>>
>>>   The macro can't work reliably when one of the CLASSES is class
>>>   containing registers from multiple units (SSE, MMX, integer).  We
>>>   avoid this by never combining those units in single alternative in
>>>   the machine description. Ensure that this constraint holds to avoid
>>>   unexpected surprises.
>>>
>>> So, this indicates that we shouldn't be using INT_SSE_REGS for a reload
>>> class at all, and I expect that at the moment we don't. With the patch,
>>> the new find_valid_class_1 discovers INT_SSE_REGS as the best class for
>>> the register to hold the SYMBOL_REF, leading to the failed assert.
>
> Actually. existing LIMIT_RELOAD_CLASS is way too simple to handle all
> issues with mixed register sets. Looking at ix86_hard_regno_mode_ok,
> we have problems with DI and SI mode, which can go int XMM and GENERAL
> regs, and SF and DF mode, which can go into XMM, FLOAT and GENERAL
> regs, depending on the availability of units.
>
> Attached (RFC) patch handles this limitation by limiting multiple
> register set modes to the "natural mode" register set, i.e. DI and SI
> modes will always return GENERAL_REGS, DF and SF will return either
> SSE_REGS, or FLOAT_REGS or GENERAL_REGS. Please note, that we don't
> want to widen i.e. CREG or ADREG narrow classes to full GENERAL_REGS.
>
> The patch also improves Q_REGS selection in the same way, and adds a
> couple of missing registers to various register sets, so the macro
> works as expected.

I have committed the patch to mainline SVN. The testcase options were
adjusted to really fail for all default cases of mpfmath on x86. Also,
the testcase that fails on 64bit targets was added.

2012-08-09  Uros Bizjak  <ubizjak@gmail.com>

	* config/i386/i386.h (LIMIT_RELOAD_CLASS): Return preferred
	single unit register class for classes that contain registers form
	multiple units.
	(REG_CLASS_CONTENTS): Add missing "frame" register to FLOAT_INT_REGS,
	INT_SSE_REGS and FLOAT_INT_SSE_REGS register classes.

testsuite/ChangeLog:

2012-08-09  Uros Bizjak  <ubizjak@gmail.com>

	* gcc.c-torture/compile/20120727-1.c (dg-options): Add -mfpmath=387
	for x86 targets.
	* gcc.c-torture/compile/20120727-2.c: New test.

Re-tested on x86_64-pc-linux-gnu {,-m32} and committed.

Uros.
-------------- next part --------------
Index: config/i386/i386.h
===================================================================
--- config/i386/i386.h	(revision 190254)
+++ config/i386/i386.h	(working copy)
@@ -1298,9 +1298,9 @@
 { 0x1fe00100,0x1fe000 },		/* FP_TOP_SSE_REG */		\
 { 0x1fe00200,0x1fe000 },		/* FP_SECOND_SSE_REG */		\
 { 0x1fe0ff00,0x1fe000 },		/* FLOAT_SSE_REGS */		\
-   { 0x1ffff,  0x1fe0 },		/* FLOAT_INT_REGS */		\
-{ 0x1fe100ff,0x1fffe0 },		/* INT_SSE_REGS */		\
-{ 0x1fe1ffff,0x1fffe0 },		/* FLOAT_INT_SSE_REGS */	\
+  { 0x11ffff,  0x1fe0 },		/* FLOAT_INT_REGS */		\
+{ 0x1ff100ff,0x1fffe0 },		/* INT_SSE_REGS */		\
+{ 0x1ff1ffff,0x1fffe0 },		/* FLOAT_INT_SSE_REGS */	\
 { 0xffffffff,0x1fffff }							\
 }
 
@@ -1378,15 +1378,28 @@
 
 /* Place additional restrictions on the register class to use when it
    is necessary to be able to hold a value of mode MODE in a reload
-   register for which class CLASS would ordinarily be used.  */
+   register for which class CLASS would ordinarily be used.
 
-#define LIMIT_RELOAD_CLASS(MODE, CLASS) 			\
-  ((MODE) == QImode && !TARGET_64BIT				\
-   && ((CLASS) == ALL_REGS || (CLASS) == GENERAL_REGS		\
-       || (CLASS) == LEGACY_REGS || (CLASS) == INDEX_REGS)	\
-   ? Q_REGS							\
-   : (CLASS) == INT_SSE_REGS ? GENERAL_REGS : (CLASS))
+   We avoid classes containing registers from multiple units due to
+   the limitation in ix86_secondary_memory_needed.  We limit these
+   classes to their "natural mode" single unit register class, depending
+   on the unit availability.
 
+   Please note that reg_class_subset_p is not commutative, so these
+   conditions mean "... if (CLASS) includes ALL registers from the
+   register set."  */
+
+#define LIMIT_RELOAD_CLASS(MODE, CLASS)					\
+  (((MODE) == QImode && !TARGET_64BIT					\
+    && reg_class_subset_p (Q_REGS, (CLASS))) ? Q_REGS			\
+   : (((MODE) == SImode || (MODE) == DImode)				\
+      && reg_class_subset_p (GENERAL_REGS, (CLASS))) ? GENERAL_REGS	\
+   : (SSE_FLOAT_MODE_P (MODE) && TARGET_SSE_MATH			\
+      && reg_class_subset_p (SSE_REGS, (CLASS))) ? SSE_REGS		\
+   : (X87_FLOAT_MODE_P (MODE)						\
+      && reg_class_subset_p (FLOAT_REGS, (CLASS))) ? FLOAT_REGS		\
+   : (CLASS))
+
 /* If we are copying between general and FP registers, we need a memory
    location. The same is true for SSE and MMX registers.  */
 #define SECONDARY_MEMORY_NEEDED(CLASS1, CLASS2, MODE) \
Index: testsuite/gcc.c-torture/compile/20120727-1.c
===================================================================
--- testsuite/gcc.c-torture/compile/20120727-1.c	(revision 190254)
+++ testsuite/gcc.c-torture/compile/20120727-1.c	(working copy)
@@ -1,3 +1,5 @@
+/* { dg-options "-mfpmath=387" { target { i?86-*-* x86_64-*-* } } } */
+
 union {
   char *p;
   float f;
@@ -7,5 +9,5 @@
 f (void)
 {
   u.p = "";
-  u.f += 1.1;
+  u.f += 1.1f;
 }
Index: testsuite/gcc.c-torture/compile/20120727-2.c
===================================================================
--- testsuite/gcc.c-torture/compile/20120727-2.c	(revision 0)
+++ testsuite/gcc.c-torture/compile/20120727-2.c	(working copy)
@@ -0,0 +1,13 @@
+/* { dg-options "-mfpmath=387" { target { i?86-*-* x86_64-*-* } } } */
+
+union {
+  char *p;
+  double d;
+} u;
+
+void
+f (void)
+{
+  u.p = "";
+  u.d += 1.1;
+}