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

Vaidas Gasiunas vaidas.gasiunas at sap.com
Mon Dec 8 06:42:19 PST 2014


Hi Quentin,

Thanks a lot for the comments. I added the option to turn-off the optimization and fixed formatting and comments.

Best regards,
Vaidas

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

================
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) {
----------------
qcolombet wrote:
> 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?
The set iterator supports only const access to elements, therefore I need S = segmentAt(I) for const_cast.

Set iterators are read-only, because changing a set element may potentially affect is order in the set. Thus, we must be careful to change the segments in a way that preserves their order. Here it is no problem, because these algorithms were designed to take care of correct ordering.

================
Comment at: lib/CodeGen/LiveInterval.cpp:232
@@ +231,3 @@
+
+#define CALC_LIVE_RANGE_UTIL_VECTOR_BASE \
+   CalcLiveRangeUtilBase<CalcLiveRangeUtilVector, LiveRange::iterator, LiveRange::Segments>
----------------
qcolombet wrote:
> Couldn’t this be a typedef?
Yes, it works. I just need to forward declare CalcLiveRangeUtilVector .

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

================
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) {
----------------
qcolombet wrote:
> Capital letter and period to make a sentence.
Done

================
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);
----------------
qcolombet wrote:
> 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.
Done

http://reviews.llvm.org/D6013






More information about the llvm-commits mailing list