Patchwork [RFC,i386] : Improve LIMIT_RELOAD_CLASSES [was: Reload/i386 patch for secondary memory vs subregs]

login
register
mail settings
Submitter Uros Bizjak
Date Aug. 4, 2012, 12:26 p.m.
Message ID <CAFULd4aaLyD36+9aELu_GTj9qJ+oKLXZFvdom2ApkNK8-FHmsg@mail.gmail.com>
Download mbox | patch
Permalink /patch/175105/
State New
Headers show

Comments

Uros Bizjak - Aug. 4, 2012, 12:26 p.m.
On Fri, Aug 3, 2012 at 10:37 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.

2012-08-04  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.

Patch was bootstrapped on x86_64-pc-linux-gnu {,-m32}. Regression test
is in process.

Uros.

Patch

Index: i386.h
===================================================================
--- i386.h	(revision 190141)
+++ i386.h	(working copy)
@@ -1297,9 +1297,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 }							\
 }
 
@@ -1377,14 +1377,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))
+   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) \