[PATCH] Inverse post-order traversal for LiveVariables analysis, to recover the performance after r214064

[Artyom Skrobov]

>>> It is true that this inverts the order, but I don't think this
>>> definition of po_iterator is used anywhere except the constructor
>>> for PostOrderInverseCFGView.
>> 
>> You're right, it's not; but equally the definition of
>> PostOrderCFGView::po_iterator isn't used anywhere except the
>> constructor for PostOrderCFGView: it's declared privately,
>> and it's not used by any PostOrderCFGView consumers.
>
> Ok, that makes sense now, but it makes me feel the APIs themselves
> are a bit misleading/confusing.  (see below)

Would it help make things a bit clearer if we move this private typedef out
of PostOrderCFGView declaration, into PostOrderCFGView constructor body?
(And the same for the new PostOrderInverseCFGView)


>>> There is no virtual definition of the 'dequeue' method, so
>>> 'dequeue' (which is declared in DataflowWorklistBase) uses
>>> the po_iterator defined in DataflowWorklistBase, which is
>>> the wrong definition for reverse analysis.
>> 
>> No, dequeue() uses PostOrderCFGView::iterator, which
>> iterates over the Blocks vector. So, the only factor
>> affecting the order of iteration is how the Blocks vector
>> is initialized in the PostOrderInverseCFGView constructor.
>
> I see it now.  From an API perspective it is very confusing,
> because the name "PostOrderCFGView" implies the iteration
> always is in that order.

Well, that's true: PostOrderCFGView always does it in the "normal" order,
and PostOrderInverseCFGView always does it in the inverse order.


> Also the comment is very misleading if we are doing a reverse
> analysis:
>
>  // Next dequeue from the initial reverse post order.  This is the
>  // theoretical ideal in the presence of no back edges.
>
> Technically we'd be doing the opposite thing if this was a
> reverse analysis.  The comment if anything is what hung me
> up on thinking it was doing the wrong thing.

The terminology here is a bit muddy, but I took it that "reverse" means
"back to forward", in the sense of using Blocks.rbegin() for the iteration
(which still applies to DataflowWorklistInverse); while "inverse" means
"bottom to top", in the sense of po_iterator visiting predecessors instead
of successors for each block. Assuming this terminology, DataflowWorklist
uses "reverse normal order", and DataflowWorklistInverse uses "reverse
inverse order"; but the comment in question remains valid in both cases.

(I guess I should add the above paragraph into a comment in
DataflowWorklist.h?)