>>> 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?) _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
