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

[Vaidas Gasiunas]

> c) What's the slowdown look like for using it all the time? Is it because we're using find instead of set::upper_bound? Where are we running into issues?

I was experimenting with it almost a year ago, so I don't remember precisely which operations were slower. I think it was not lookup, but rather some range operations like splitting or merging of ranges, that work much faster with the array. Unfortunately, I cannot try out that again with the current patch, because now only the operations that are used in the initial computation of the live range are implemented for set. 

> d) It seems like we're duplicating a lot of code to do this, is there any way you can think of to remove that duplication here?

Yes I also don't like the code duplication. This was actually the reason why I hesitated to contribute that patch. I am open to suggestions.

> b) Pull the asserts out of the else blocks.

Which ones do you mean? Note that most of the new code is a copy of analogous operations with vector.

> Also, I think for performance evaluation it would be nice to be able to use or not use it every with a command line option. That would help to figure out what is the problem with computeVirtRegs. Moreover that would allow to completely disable it if we believe it has some problem.

Note that computeVirtRegs is not really a problem, it simply does not really need that optimization, but yes it makes sense to have an switch for the optimization as whole. 

Vaidas

================
Comment at: lib/CodeGen/LiveInterval.cpp:622
@@ +621,3 @@
+VNInfo *LiveRange::segsetCreateDeadDef(SlotIndex Def,
+                                  VNInfo::Allocator &VNInfoAllocator) {
+  assert(!Def.isDead() && "Cannot define a value at the dead slot");
----------------
qcolombet wrote:
> Please try to share this code between the vector and the set versions.
> You could stick to the existing implementation (with some abstract on the iterator type) and use an abstraction for the final insertion for instance.
I don't see how it would work. Could you give some more details? Do you suggest having some adapter iterator wrapping one of the two iterators? 

================
Comment at: lib/CodeGen/LiveInterval.cpp:656
@@ +655,3 @@
+
+  if (segmentSet->empty()) {
+    return nullptr;
----------------
qcolombet wrote:
> Same thing as segsetCreateDeadDef, you should share more logic between the implementations.
> For instance, you could abstract begin(), upper_bound(), empty() and you should be able to reuse all the code.
How can I abstract them, if the two iterator types are not compatible with each other? 

================
Comment at: lib/CodeGen/LiveInterval.cpp:948
@@ +947,3 @@
+  if (LR->segmentSet != NULL) {
+    LR->addSegment(Seg);
+    return;
----------------
qcolombet wrote:
> Is this code path used during createDeadDefs and extendToUses?
It is used in LiveRangeCalc::extendToUses through LiveRangeCalc::findReachingDefs

================
Comment at: lib/CodeGen/LiveIntervalAnalysis.cpp:315
@@ -311,1 +314,3 @@
+    // flush the segment set to array and destroy it
+    LR->flushSegmentSet();
   }
----------------
qcolombet wrote:
> What about pushing the flush into computeRegUnitRange?
> 
> Even better, could we automatically decide when to do the flush internally in the LiveRange class? E.g., the first time we actually need the vector capabilities.
> That would remove this kind of explicit calls to flush that are easy to forget or misplace.
At first I put it to computeRegUnitRange, but I thought that is a more clear design if we have calls to createSegmentSet() and flushSegmentSet() at the same level.

Flushing automatically sounds good, I will try it out.

http://reviews.llvm.org/D6013