[PATCH] D64174: [DAGCombine] Do several rounds of combine for addcarry nodes.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> Changing this lead very quickly to a world of pain

Probably, yes. :)

> Topological sorting tends to be an expensive operation, it can easily go into n^2 territory

Sorting once should be linear in the number of edges, and SelectionDAG DAGs generally have few edges.  And it's not that expensive in practice; we do it twice already as part of normal compilation.  (See AssignTopologicalOrder.)

> Do you see a way to create/maintain a worklist that is ordered as expected?

isel itself is an algorithm which incrementally transforms a SelectionDAG DAG and maintains topological ordering as it works.  It visits the wrong direction for what we want, but it should be possible to reverse.  Granted, this is maybe not the best approach; most DAGCombines take a node and produce one or more new nodes, just like isel, but I'm not sure how you'd handle the ones that don't.

Or you could probably reproduce something similar without actually maintaining the full topological sort on the DAG itself.  Topological sort at the beginning, to build an initial worklist.  Then, whenever new nodes are created, keep a separate list of those nodes, and visit them in order of creation before continuing with the regular worklist.  For most combines, which visit a node, and replace it with one or more new nodes, all the new nodes should have operands which are either new nodes or nodes that have already been visited, and the results should only be used by new nodes and nodes which have not yet been visited.  So you maintain sorted visitation order for the existing nodes.  And creation order should be naturally sorted for the new nodes.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64174/new/

https://reviews.llvm.org/D64174