[PATCH] D129352: [CodeGen] Limit building time for CodeGenPrepare

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:573
       // Restart BB iteration if the dominator tree of the Function was changed
       if (ModifiedDTOnIteration)
         break;
----------------
xiangzhangllvm wrote:
> nikic wrote:
> > Before introducing limits, can we try to remove this case? That is, continue simplifying the rest of the blocks before we start a new iteration? I expect that the current implementation looks like this to avoid some iterator invalidation issues, but there's probably a way to avoid that with a bit more effort.
> Thanks a lot for reviewing this patch! but if the CFG changed, we should re-start the iteration, or the iteration may have problem.
This is what we currently do, but the current implementation has a lot of room for improvement. For example, combineToUAddWithOverflow() will set ModifiedDT even though it does not modify control flow at all! It only does it to avoid iterating over a dead instruction in the same block. This can be solved by either only restarting the iteration in the BB (rather than the whole function), or by passing in the iterator to optimizeInst() so it can be moved in such case, or similar.

For cases that actually do modify the CFG, we could use DTU instead of full DT recomputation, and we could maybe switch to a BB worklist, so new blocks can be queued, and deleted blocks removed from the worklist.

Nobody has bothered to do this yet, because it never came up as a problem. Now that it did, things should get fixed properly.


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

https://reviews.llvm.org/D129352