On Fri, Aug 3, 2012 at 10:26 PM, Bernd Schmidt <ber...@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.
