[PATCH] Speed up creation of live ranges for physical registers by using a segment set

[Quentin Colombet]

Hi Vaidas,

Thanks for the refactoring, this is looking good!

I just have a couple of nitpicks (see inline comments) and one request:
- Could you add a cl::opt to override the use of segment set?

Thanks again,
-Quentin

================
Comment at: include/llvm/CodeGen/LiveIntervalAnalysis.h:385
@@ -385,1 +384,3 @@
+        // Use segment set to speed-up initial computation of the live range.
+        RegUnitRanges[Unit] = LR = new LiveRange(/*UseSegmentSet=*/true);
         computeRegUnitRange(*LR, Unit);
----------------
For debugging purposes, I would like we have a command line option to override this behavior.
This means we will have to update flushSegmentSet accordingly or not calling it when the option is false.

================
Comment at: lib/CodeGen/LiveInterval.cpp:63
@@ +62,3 @@
+
+    // otherwise use the vector
+    iterator I = impl().find(Def);
----------------
I believe this comment is out of date.

================
Comment at: lib/CodeGen/LiveInterval.cpp:92
@@ +91,3 @@
+  VNInfo *extendInBlock(SlotIndex StartIdx, SlotIndex Kill) {
+    // otherwise use the segment vector
+    if (segments().empty())
----------------
Ditto.

================
Comment at: lib/CodeGen/LiveInterval.cpp:125
@@ +124,3 @@
+    // have the same value number, merge the two segments into one segment.
+    if (MergeTo != segments().end() && MergeTo->start <= I->end &&
+        MergeTo->valno == ValNo) {
----------------
Using ’S' instead of ‘I' may be clearer here also it does not matter for correctness.
On a second thought, do we actually need S?

================
Comment at: lib/CodeGen/LiveInterval.cpp:140
@@ +139,3 @@
+    assert(I != segments().end() && "Not a valid segment!");
+    Segment* S = segmentAt(I);
+    VNInfo *ValNo = I->valno;
----------------
Ditto.

================
Comment at: lib/CodeGen/LiveInterval.cpp:158
@@ +157,3 @@
+    if (MergeTo->end >= NewStart && MergeTo->valno == ValNo) {
+      segmentAt(MergeTo)->end = S->end;
+    } else {
----------------
Do we really need to use segmentAt?
Couldn’t we use MergeTo directly?

================
Comment at: lib/CodeGen/LiveInterval.cpp:162
@@ +161,3 @@
+      ++MergeTo;
+      Segment* MergeToSeg = segmentAt(MergeTo);
+      MergeToSeg->start = NewStart;
----------------
Ditto.

================
Comment at: lib/CodeGen/LiveInterval.cpp:232
@@ +231,3 @@
+
+#define CALC_LIVE_RANGE_UTIL_VECTOR_BASE \
+   CalcLiveRangeUtilBase<CalcLiveRangeUtilVector, LiveRange::iterator, LiveRange::Segments>
----------------
Couldn’t this be a typedef?

================
Comment at: lib/CodeGen/LiveInterval.cpp:259
@@ +258,3 @@
+
+#define CALC_LIVE_RANGE_UTIL_SET_BASE \
+   CalcLiveRangeUtilBase<CalcLiveRangeUtilSet, LiveRange::SegmentSet::iterator, LiveRange::SegmentSet>
----------------
Ditto.

================
Comment at: lib/CodeGen/LiveInterval.cpp:320
@@ +319,3 @@
+    return CalcLiveRangeUtilSet(this).createDeadDef(Def, VNInfoAllocator);
+  else
+    return CalcLiveRangeUtilVector(this).createDeadDef(Def, VNInfoAllocator);
----------------
Do not use else after return: http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

================
Comment at: lib/CodeGen/LiveInterval.cpp:469
@@ -270,1 +468,3 @@
+    return end();
   } else {
+    return CalcLiveRangeUtilVector(this).addSegment(S);
----------------
Do not use else after return: http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

================
Comment at: lib/CodeGen/LiveInterval.cpp:480
@@ +479,3 @@
+    return CalcLiveRangeUtilSet(this).extendInBlock(StartIdx, Kill);
+  else
+    return CalcLiveRangeUtilVector(this).extendInBlock(StartIdx, Kill);
----------------
Do not use else after return: http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

================
Comment at: lib/CodeGen/LiveInterval.cpp:874
@@ +873,3 @@
+  // fall back to the regular add method if the live range
+  // is using the segment set instead of the segment vector
+  if (LR->segmentSet != nullptr) {
----------------
Capital letter and period to make a sentence.

================
Comment at: lib/CodeGen/LiveIntervalAnalysis.cpp:299
@@ -296,1 +298,3 @@
+          // Use segment set to speed-up initial computation of the live range.
+          LR = RegUnitRanges[Unit] = new LiveRange(/*UseSegmentSet=*/true);
           NewRanges.push_back(Unit);
----------------
For debugging purposes, I would like we have a command line option to override this behavior.
This means we will have to update flushSegmentSet accordingly or not calling it when the option is false.

http://reviews.llvm.org/D6013