[PATCH] D33587: [DAGCombine] Do several rounds of combine.

[Amaury SECHET via Phabricator via llvm-commits]

deadalnix marked an inline comment as done.
deadalnix added a comment.

I'm not sure how processing nodes bottom up really helps. Problems arise when you want to use patterns of depth > 2 because then direct parent.child are not processed again, even though such pattern may now be available. It seems to me that both top/down and bottom/up approaches would suffers from the same problem, but maybe there is something I'm missing.



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:1437
+      // this won't repeatedly process the same operand.
+      CombinedNodes.insert(N);
+      for (const SDValue &ChildN : N->op_values())
----------------
craig.topper wrote:
> Don't you need to reset CombineNodes on each iteration?
Both would be correct but semantic obviously differ. Let me investigate this. Good catch.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D33587