[llvm-commits] [llvm] r59266 - in /llvm/trunk: include/llvm/Target/TargetLowering.h lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86RegisterInfo.td

[Dale Johannesen]

On Nov 16, 2008, at 9:23 PMPST, Evan Cheng wrote:

> I have mixed feelings about this solution. That said, why does isel
> need to pre-assign registers if the new register class ensures the
> right allocation is done?
>
> Evan

ISel (or, if you prefer, the FE) preassigns registers when there is  
only one register that fits the constraint, as with "d".  This is no  
different except that two registers are involved instead of one.   
Handling "d" and "A" similarly is clearly right.

IMO doing fixed-register allocations early is the right idea; this  
should prevent RAs from assigning conflicting registers across the asm  
and having to spill them, at least sometimes, and there's no upside to  
delaying the assignment.  (Early allocation for tied constraints is  
inferior, though; that's a matter of the RAs not being able to do them  
right.)

> On Nov 16, 2008, at 4:48 PM, Dale Johannesen <dalej at apple.com> wrote:
>
>>
>> On Nov 15, 2008, at 7:32 PM, Chris Lattner wrote:
>>>>> Does llc slow down
>>>>> at "-O0" by adding another register class?
>>>>
>>>>> Finally, how does this fix the bug?  Is isel still doing the
>>>>> 'register
>>>>> allocation' of the 'A' constraint, or is it turning it into vregs
>>>>> from
>>>>> the GRAD reg class?  I thought the bug was that the isel regalloc
>>>>> was
>>>>> assigning the tied register to EAX and then it just wasn't
>>>>> available
>>>>> when handling the A constraint.
>>>>
>>>>
>>>> ISel is allocating the registers for both arguments before and  
>>>> after
>>>> the patch.  Since 'A' and the tied register were both previously
>>>> treated as C_RegisterClass, they were allocated in argument order,
>>>> which led to the problem you say.  Now that 'A' is marked as  
>>>> using a
>>>> specific hard register, it gets allocated before the tied register,
>>>> which then knows it cannot use EAX and EDX.
>>>
>>> Ok.  My problem with this is that adding register classes to the
>>> target *just* to support inline asm seems bad.  Inline is an evil
>>> menace to the code generator, and IMO it should have as minimal
>>> impact
>>> on it as possible.
>>
>> I don't like inline asm either.  I don't know a compiler person who
>> does.  That's irrelevant; we need to support it, and supporting it
>> badly is not going to make it go away.  I'm strongly of the opinion
>> this is the cleanest and most elegant solution for this problem.
>>
>>> I'd be much happier with changing the SDISel code to do an extra  
>>> pass
>>> over the input/output list of the asm to look for these and handle
>>> them early if that would solve the problem.  This way, only inline
>>> asm
>>> pays the compile-time cost, and the targets don't have to sprout new
>>> reg classes to match the GCC constraint letters.  Do you think this
>>> is
>>> a reasonable approach?
>>
>> It is already doing a pass to handle fixed-register constraint
>> letters.  In other cases these are marked specially by the front end,
>> as "{eax}".  I did consider extending that notation to allow "{eax
>> +edx}", but the register class solution is much simpler and cleaner
>> than changing all front ends, plural.  Further, it handles a target-
>> dependent problem in the target code, where it belongs.
>>
>> I really don't understand your objection to adding register classes.
>> This is free aside from an unnoticeable amount of compile time, and a
>> good general mechanism.  Actually I think it would be preferable to
>> handle the other fixed-register constraints, like "d", with register
>> classes and rip all that conversion crap out of the front end (or
>> ends, if it's gotten into clang already).  Not that I'm planning to  
>> do
>> that.
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits