[llvm-commits] [llvm] r59266 - in /llvm/trunk: include/llvm/Target/TargetLowering.h lib/Target/X86/X86ISelLowering.cpp lib/Target/X86/X86RegisterInfo.td
clattner at apple.com
Sat Nov 15 21:32:47 CST 2008
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
>> allocation' of the 'A' constraint, or is it turning it into vregs
>> 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?
More information about the llvm-commits