[llvm-commits] [PATCH] Added topological graph sort routine to llvm/ADT

[Delesley Hutchins]

Good point.  Yes, the RPO iterator (not PO iterator -- my bad) does
seem to do what we need.  The ordering is slightly different, but it
obeys the same constraints.  Moreover, the efficiency of RPO is
somewhat better, since it does not attempt to track depth so
precisely.  I am withdrawing the patch.

  -DeLesley

On Thu, Aug 18, 2011 at 10:17 AM, Chandler Carruth <chandlerc at google.com> wrote:
> On Thu, Aug 18, 2011 at 10:03 AM, Delesley Hutchins <delesley at google.com>
> wrote:
>>
>> AFAICT, the RPO iterator does not do what we need.  For example,
>> assume we have the following graph (pardon the bad ASCII graphics):
>>
>>   /---> (2) ----------\
>> (1)                     --> (5)
>>   \---> (3) ---> (4)--/
>>
>> I believe the RPO iterator will traverse this graph as 5-2-4-3-1,
>> which is a standard depth-first post-order traversal.
>>
>> What we want is a topological traversal, which is 1-2-3-4-5; every
>> predecessor of a node must be traversed before the node itself.
>
> What about the reverse of the RPO violates this constraint? 1-3-4-2-5 looks
> like it satisfies your constraint. I think Jakub is aware that we would have
> to reverse the order, and was just claiming that such a reversed order
> satisfied the constraints of a topological ordering.
>
>>
>> Notice that a topological traversal is neither depth-first (that would
>> be 1-2-5-3-4), nor breadth-first (1-2-3-5-4); we want to ensure that 2
>> and 4 are traversed before 5.
>>
>> This patch does three things to achieve the topological traversal:
>> (1) Depth-first traversal to identify back-edges and assign depths.
>> (2) Sort nodes according to depth.
>> (3) Traversal of the sorted list of nodes.
>>
>>  -DeLesley
>>
>> On Wed, Aug 17, 2011 at 10:00 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk>
>> wrote:
>> >
>> > On Aug 17, 2011, at 9:17 PM, Chandler Carruth wrote:
>> >
>> > On Wed, Aug 17, 2011 at 8:56 PM, Jakob Stoklund Olesen <stoklund at 2pi.dk>
>> > wrote:
>> >>
>> >> On Aug 17, 2011, at 3:52 PM, Delesley Hutchins wrote:
>> >>
>> >> > This patch adds a topological sort routine for indexed graphs to
>> >> > llvm/ADT.  This sort routine is currently used to traverse CFGs in
>> >> > Clang when doing thread safety analysis (patch to be released
>> >> > shortly).
>> >>
>> >> Could you explain why we need this in addition to
>> >> include/llvm/ADT/PostOrderIterator.h?
>> >
>> > FYI, I'm not terribly familiar w/ PostOrderIterator (so maybe it would
>> > actually serve here), but JSYK, the motivating use case is in a Clang
>> >
>> > patch: http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20110815/045262.html
>> >
>> > I see.
>> > AFAICT, the existing RPO iterator will give you the exact same ordering
>> > of
>> > blocks, at least it will give you a topo-sort of the graph after
>> > removing
>> > back-edges.
>> > You can detect back-edges by keeping track of visited blocks in a
>> > BitVector
>> > as you iterate through the RPO.
>> > /jakob
>> >
>>
>>
>>
>> --
>> DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315
>
>



-- 
DeLesley Hutchins | Software Engineer | delesley at google.com | 505-206-0315