[PATCH] Add a case to LiveIntervalAnalysis::HandleMoveUp

[Andrew Trick]

On Mar 5, 2013, at 11:20 AM, Vincent Lejeune <vljn at ovi.com> wrote:

> I reworked my patch a bit so that it now updates the read-undef flags.
> I planned to let MachineScheduler do it however updating the flag inside HandleMove allows to spot error quickier.
> 
> Can you have a look at it please ? 

Hopefully Jakob can comment. I just want to point out that if the scheduler does anything other than simple movement of instructions, we should have a more robust API. I would much rather use something like the LiveIntervals::repairIntervalsInRange. I'm just not sure yet if that covers all the corner cases we'll hit in the scheduler.

-Andy

> ----- Mail original -----
>> De : Andrew Trick <atrick at apple.com>
>> À : Vincent Lejeune <vljn at ovi.com>
>> Cc : Jakob Stoklund Olesen <stoklund at 2pi.dk>; "llvm-commits at cs.uiuc.edu" <llvm-commits at cs.uiuc.edu>
>> Envoyé le : Mercredi 13 février 2013 0h10
>> Objet : Re: [PATCH] Add a case to LiveIntervalAnalysis::HandleMoveUp
>> 
>> 
>> On Feb 9, 2013, at 4:14 PM, Vincent Lejeune <vljn at ovi.com> wrote:
>> 
>>> Hi Jakob,
>>> 
>>> I'd like to reorder subregisters as part of pre RA scheduling machine 
>> instruction, in order to maximise bundling opportunities later.
>>> In R600 there are 128 bits registers, Tn_XYZW, and 4 more times 32 bits 
>> registers, Tn_X, Tn_Y, TnZ and Tn_W,  with n between 0 and 127 in both case.
>>> The Tn_X, Tn_Y, Tn_Z and Tn_W register are also subregister of the Tn_XYZW 
>> register. They map to what the R600 doc calls "channel", that is T1_X 
>> is 
>>> channel Y of register 1.
>>> The main rule to bundle instructions is that 2 instructions in a bundle 
>> must write to a different channel (whose determine the ALU which will execute 
>> it). 
>>> 2 instructions in a bundle don't have to write to the same 128 bits 
>> register.
>>> 
>>> I have the following machine instructions list :
>>>     %vreg234 = COPY (...);
>>> 
>>> ...
>>> 
>>>      %vreg403:sel_x<def,read-undef> = MULADD_IEEE_eg (...); 
>>>      %vreg403:sel_y<def> = MULADD_IEEE_eg (...); 
>>> 
>>>      %vreg234:sel_x<def> = MULADD_IEEE_eg (...);
>>>      %vreg234:sel_y<def,read-undef> = MULADD_IEEE_eg (...); 
>>> 
>>>      %vreg234:sel_z<def,read-undef> = MULADD_IEEE_eg (...); 
>>> 
>>>      %vreg234:sel_w<def,read-undef> = MULADD_IEEE_eg (...); 
>>>      %vreg27:sel_z<def> = COPY %vreg234:sel_z; 
>>>      %vreg27:sel_w<def> = COPY %vreg234:sel_z; 
>> 
>> Somewhat independent from this patch, another issue is that the scheduler, 
>> probably via handleMove, needs to update the read-undef flag so it always occurs 
>> on the first subreg def. I think the flag is mainly for verification.
>> 
>> -Andy
>> 
>>> If I don't change order of subreg def, this makes 3 bundles at least :
>>> (%vreg403:sel_x = ..., %vreg403:sel_y = ...)
>>> (%vreg234:sel_x = ...,%vreg234:sel_y = ...,%vreg234:sel_z = 
>> ...,%vreg234:sel_w = ...) 
>>> (%vreg27:sel_z = ..., %vreg27:sel_w = ...)
>>> 
>>> It is possible to use only 2 bundles if it is possible to reorder subreg 
>> definition :
>>> 
>>>      %vreg403:sel_x<def,read-undef> = MULADD_IEEE_eg (...); 
>>>      %vreg403:sel_y<def> = MULADD_IEEE_eg (...); 
>>> 
>>>      %vreg234:sel_z<def,read-undef> = MULADD_IEEE_eg (...); 
>>>      %vreg234:sel_w<def,read-undef> = MULADD_IEEE_eg (...); 
>>> 
>>> and
>>> 
>>> 
>>>      %vreg234:sel_x<def> = MULADD_IEEE_eg (...); 
>>>      %vreg234:sel_y<def,read-undef> = MULADD_IEEE_eg (...); 
>>> 
>>>      %vreg27:sel_z<def> = COPY %vreg234:sel_z;
>>>      %vreg27:sel_w<def> = COPY %vreg234:sel_z; 
>>> 
>>> 
>>> which means moving the 5th and 6th instructions right before the 3rd one).
>>> 
>>> However LiveIntervalAnalysis::HandleMove does not allow this move and 
>> crashes with an
>>> assertion failure (either I->start <= I->end or I->end <= 
>> llvm::next(I)->start is not verified ).
>>> With this patch I'd like to make HandleMove support such modification
>>> (I'd like to preserve LiveIntervalAnalysis before RA).
>>> 
>>> 
>>> Thank,
>>> Vincent
>>> 
>>> 
>>> ----- Mail original -----
>>>> De : Jakob Stoklund Olesen <stoklund at 2pi.dk>
>>>> À : Vincent Lejeune <vljn at ovi.com>
>>>> Cc : llvm-commits at cs.uiuc.edu
>>>> Envoyé le : Samedi 9 février 2013 18h19
>>>> Objet : Re: [PATCH] Add a case to LiveIntervalAnalysis::HandleMoveUp
>>>> 
>>>> 
>>>> On Feb 8, 2013, at 2:31 PM, Vincent Lejeune <vljn at ovi.com> wrote:
>>>> 
>>>>>    Some target like R600 may want to swap LiveRange order for a same 
>> register.
>>>>>    This happens for instance if several subregisters can be 
>> independantly 
>>>> written
>>>>>    to ; for instance in the case of R600, a 128 bits register is 
>> composed of 4 
>>>> 32
>>>>>    bits subregisters that have no write order restriction. To 
>> maximise VLIW 
>>>> bundle
>>>>>    size and thus increase parrallelism the R600 target may swap 
>> subreg 
>>>> definitions.
>>>>>    This patch allows LiveIntervals to support such changes.
>>>> 
>>>> Hi Vincent,
>>>> 
>>>> Thanks for working on this, I think it is a useful feature.
>>>> 
>>>> The live intervals maintain an SSA form on the side - all live ranges 
>> must be 
>>>> dominated by their VNInfo def. That makes it a bit complicated to move 
>> defs 
>>>> across other defs.
>>>> 
>>>> Could you give an example of what you're trying to do, including 
>>>> before/after live intervals? Make sure your virtual register is 
>> live-out of the 
>>>> basic block.
>>>> 
>>>> (And please use proper punctuation in your comments).
>>>> 
>>>> Thanks,
>>>> /jakob
>>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> <0001-Add-a-case-to-LiveIntervalAnalysis-HandleMove.patch>