[PATCH] Add a case to LiveIntervalAnalysis::HandleMoveUp

Vincent Lejeune vljn at ovi.com
Tue Sep 17 08:24:54 PDT 2013


Hi Jakob,

Now that R600 backend properly packetizes instructions and supports all vliw modifications in the
R600 family I have some more reliable test to check correctness of this change.
I updated the patch ; I didn't find a lot of place to add assert (I'm relying on the LiveInterval::verify() function calls
made by the MachineScheduler).
The patch here : http://cgit.freedesktop.org/~vlj/llvm/commit/?h=vliw5&id=80a1d4220f501afaa714ad076422bd332375e616
uses these changes and provides some tests.
The LiveInterval refactoring patch series may be impacted by this patch (the goal of the serie is to track subreg liveness
which is directly related to what I need) so I'm adding Matthias Braun to the recipient lists.

Vincent





----- Mail original -----
> De : Jakob Stoklund Olesen <stoklund at 2pi.dk>
> À : Vincent Lejeune <vljn at ovi.com>
> Cc : Andrew Trick <atrick at apple.com>; Commit Messages and Patches for LLVM <llvm-commits at cs.uiuc.edu>
> Envoyé le : Lundi 15 avril 2013 20h42
> Objet : Re: [PATCH] Add a case to LiveIntervalAnalysis::HandleMoveUp
> 
> 
> On Apr 14, 2013, at 7:03 AM, Vincent Lejeune <vljn at ovi.com> wrote:
> 
>>  Hi Jakob,
>> 
>>  can I have a review of the first patch ? Apparently It doesn't bring 
> regressions,
>>  and I think I handled Live out value the way you mentionned in your 
> previous mail.
> 
> Hi Vincent,
> 
> This construct confuses me a bit:
> 
> +    switch (KeptValue) {
> +    case 0:
> ...
> +    case 1:
>> +    default:
> +      llvm_unreachable("Wrong KeptValue");
> +    }
> 
> Why not pass a bool? Better yet, since you're always passing a constant 
> KeptValue, why not have different functions?
> 
> Please don't use pointers as if they were iterators.
> 
> Please use proper punctuation in comments.
> 
> I am also concerned about the lack of assertions, it seems unlikely that this 
> code is unable to produce invalid intervals.
> 
> Thanks,
> /jakob
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-a-case-to-LiveIntervalAnalysis-HandleMove.patch
Type: text/x-patch
Size: 9067 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130917/a1ff50af/attachment.bin>


More information about the llvm-commits mailing list