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

[Nirav Dave via Phabricator via llvm-commits]

niravd added a comment.

My concern about this that DAGCombine is a relatively expensive and doing it 3 times will make a nontrivial difference in large vector compute blocks. At the very minimum we'd want to disable the most expensive merge operations for most of the passes (store merge and maybe some vector combines).

The only reason this is necessary is that some transforms rely on looking deeper than it's operands to decide if it's valid which means it's triggering condition may change without it being put on the worklist. Last time this was beign discussed I suggested changing the key node in the such transforms so they were either the earliest or a user of the earliest node so DAG changes would happen before the transform was no longer considered.

Maybe we could do the dual and more aggressively add user nodes to the worklist on a combine. I've looked at a few test cases on the rebased patch and it initial glance at the debug trace seems to indicate are both due to a change that enables a SimplifyDemandedBits-based combine from a node a few steps deep. If we could add further descendants in those cases (or alternatively recenter the SimplifyBits combines on computationally earlier nodes) we may be able to fold this back down to one pass with only marginal additional work.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D33587