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

[Xiang Zhang via Phabricator via llvm-commits]

xiangzhangllvm 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;
----------------
nikic wrote:
> 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.
> and we could maybe switch to a BB worklist, so new blocks can be queued, and deleted blocks removed from the worklist.

Oh, That is very good suggestion !
Let's first break the N! Algorithm complexity .

> For cases that actually do modify the CFG, we could use DTU instead of full DT recomputation,
According to my observation, the DT recomputation didn't consume too much time. Anyway we can refine it if one day we encounter such case. 

Thank you so much!


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

https://reviews.llvm.org/D129352