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

[Chris Lattner]

On Nov 14, 2008, at 10:02 AM, Dale Johannesen wrote:
>> Did you measure the compile time impact of this?
>
> No.  Surely unreferenced register classes aren't that inefficient?

It depends on if there are algorithms that iterate over reg classes  
etc.  I have no reason to believe that this is an actual problem, and  
Evan seems to think it isn't.  Measuring is good though :).

>> 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'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?

-Chris