[PBQP] Use a local bit-matrix to speedup searching an edge in the graph

[Arnaud A. de Grandmaison]

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.

 

Cheers,

Arnaud

 

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

 

Hi Arnaud,

 

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 B

|   <- 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.

 

- Lang.

 

 

On Wed, Mar 4, 2015 at 9:05 AM, Arnaud A. de Grandmaison <arnaud.degrandmaison at arm.com> wrote:

Hi Lang,

 

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.

 

Cheers,

--

Arnaud A. de Grandmaison

 

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150305/23a2291b/attachment.html>