This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Reload/i386 patch for secondary memory vs subregs
On Fri, Aug 3, 2012 at 10:26 PM, Bernd Schmidt <bernds@codesourcery.com> wrote:
> On 08/03/2012 08:50 PM, Uros Bizjak wrote:
>> Hello!
>>
>>> Bootstrapped and tested on i686-linux. It's also been in several of our
>>> internal trees, going back to even 4.4-based ones IIRC, and has had
>>> testing for several architectures. Ok for the i386 part? I intend to
>>> check the reload bits in soon if there are no objections.
>>
>> Index: gcc/config/i386/i386.h
>> ===================================================================
>> --- gcc/config/i386/i386.h (revision 189284)
>> +++ gcc/config/i386/i386.h (working copy)
>> @@ -1376,7 +1376,8 @@ enum reg_class
>> ((MODE) == QImode && !TARGET_64BIT \
>> && ((CLASS) == ALL_REGS || (CLASS) == GENERAL_REGS \
>> || (CLASS) == LEGACY_REGS || (CLASS) == INDEX_REGS) \
>> - ? Q_REGS : (CLASS))
>> + ? Q_REGS \
>> + : (CLASS) == INT_SSE_REGS ? GENERAL_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. */
>>
>> Is this correctness or performance issue?
>>
>> SSE regs can hold SImode or DImode values, so it looks to me that this
>> limitation is true only for QI and HI modes.
>>
>> Can you perhaps explain the reason for this limitation a bit more?
>
> 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.
Thanks for the explanation!
This part is OK then, but please add the comment.
BTW: Perhaps we should use MAYBE_INTEGER_CLASS_P here?
Uros.