[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 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.