[PATCH] Add a case to LiveIntervalAnalysis::HandleMoveUp

Jakob Stoklund Olesen stoklund at 2pi.dk
Mon Apr 15 11:42:13 PDT 2013


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





More information about the llvm-commits mailing list