[llvm-commits] [llvm] r98007 - /llvm/trunk/lib/CodeGen/MachineCSE.cpp

[Jakob Stoklund Olesen]

On Mar 8, 2010, at 7:17 PM, Evan Cheng wrote:

> 
> On Mar 8, 2010, at 7:14 PM, Jakob Stoklund Olesen wrote:
> 
>> 
>> On Mar 8, 2010, at 6:53 PM, Evan Cheng wrote:
>> 
>>> 
>>> On Mar 8, 2010, at 5:23 PM, Evan Cheng wrote:
>>> 
>>>> 
>>>> On Mar 8, 2010, at 4:57 PM, Jakob Stoklund Olesen wrote:
>>>> 
>>>>> 
>>>>> You could add "|| MRI->getRegClass(Reg)->hasSubClass(MRI->getRegClass(SrcReg))" without getting too complicated. That could help a bit with the few x86_64 subclasses as well.
>>>>> 
>>>> 
>>>> I don't think it's necessary here. It's doing local coalescing here to avoid missing some obvious cse opportunities. If sub-reg class is involved, then it's already too complicated.
>>> 
>>> Actually I do need to make it more complicated. But then BlackFin hates me (I wonder if it's related to its author).
>> 
>> Hehe. Blackfin not hating you is a good sign that you got your register classes right ;-)
>> 
>> Basic register class algebra: If an instruction operand specifies register class X, a virtual register must belong to X or a subclass of X.
>> 
>> If you want to eliminate a copy "%x = mov %y", the resulting register must make the users of %x and %y happy at the same time. That means the new register must belong to the intersection of X and Y, the register classes of %x and %y.
>> 
>> The function getCommonSubClass(X, Y) finds the largest existing register class that is a subclass of both X and Y. If getCommonSubClass returns NULL, the classes are disjoint, and you cannot eliminate the copy - no register would be legal.
>> 
>> I think Blackfin is the only target with disjoint register classes - maybe that is why it hates you.
> 
> I don't think that's the issue here. For example, 2009-08-15-LiveIn-SubReg.ll:
> Coalescing: %reg1031<def> = MOVE %reg1028
> *** to: %reg1030<def> = SETEQdd %reg1027, %reg1031
> 
> *** Bad machine code: Illegal virtual register for instruction ***
> - function:    foo
> - basic block: entry 0x2036ee4 (BB#0)
> - instruction: %reg1030<def> = SETEQdd %reg1027, %reg1028
> - operand 2:   %reg1028
> Expected a D register, but got a GR register

Sure. D is a subclass of GR.

%reg1028 is GR
%reg1031 is D

When you coalesce the resulting register class should be getCommonSubClass(GR, D) = D, but you are using %reg1028 which is GR - too big.

The copy was probably created by InstrEmitter::AddRegisterOperand to make the register classes match.

> I'm going to commit my changes to machine cse (which is not turned on yet). Can you take a look at it? Thanks.

Sure.