[PBQP] Use a local bit-matrix to speedup searching an edge in the graph
Arnaud A. de Grandmaison
arnaud.degrandmaison at arm.com
Thu Mar 5 01:26:44 PST 2015
Patch committed at r231360.
I also thought about this case and was a bit puzzled to see it did not trigger. I’ll also try it again, just in case I missed something.
From: Lang Hames [mailto:lhames at gmail.com]
Sent: 05 March 2015 05:31
To: Arnaud De Grandmaison
Cc: Commit Messages and Patches for LLVM
Subject: Re: [PBQP] Use a local bit-matrix to speedup searching an edge in the graph
This looks great. Please go ahead and commit. :)
I'm very surprised to hear that the llvm_unreachable never triggered. I will have to have a closer look at that. The Interference::apply algorithm is derived from Linear Scan. It walks the segments of the live-intervals of the function according to their starting points and maintains a set (the active set) of the live segments at the current point. The list of segments whose start point is further down in the function is called the inactive set. A duplicate interference detection should occur any time two intervals have separate overlapping segments. E.g.:
| <- A (segment 1) joins active set.
| | <- B (segment 1) joins active set, interference between A and B detected.
<- A and B both leave the active set.
| <- B (segment 2) joins the active set.
| | <- A (segment 2) joins the active set, interference between A and B (re)detected.
It is the second detection that the findEdge call is guarding against.
I'll see if I can replicate your results in the next few days. I want to revisit this code anyway: I have a suspicion that there are further performance improvements to be had, though I don't know if they'll be significant.
On Wed, Mar 4, 2015 at 9:05 AM, Arnaud A. de Grandmaison <arnaud.degrandmaison at arm.com> wrote:
The attached patch addresses a FIXME in RegAllocPBQP.cpp, in the Interference class when building the graph.
This indeed improves compilation time when building llvm+clang+lldb.
However, I wonder if this check+continue for an already seen edge should exist at all.
I may have missed something in my reading / understanding of Interference::apply(), but I do not think the algorithm can exhibit such a case --- or it would be an error, which we should trap with an assert, either in Interference::apply, or even better in Graph::addEdge. I replaced the continue statement with an llvm_unreachable, and it never triggered while building llvm+clang+lldb (granted, this is not a proof, just an hint). Removing this check+continue brings yet another nice speedup, making pbqp build llvm+clang+lldb slightly faster than the default allocator in release mode.
Arnaud A. de Grandmaison
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits