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

[Xiang Zhang via Phabricator via llvm-commits]

xiangzhangllvm added a comment.

In D129352#3642318 <https://reviews.llvm.org/D129352#3642318>, @yubing wrote:

> if GEP and Gather/Scatter are not in the same bb, we don't do optimize GEP in the first round. When it is second round and GEP and Gather/Scatter are put in the same bb, your code will let the GEP's optimization not happen.
>
>   // If the GEP and the gather/scatter aren't in the same BB, don't optimize.
>   // FIXME: We should support this by sinking the GEP.
>   if (MemoryInst->getParent() != GEP->getParent())
>     return false;

Let me try to find a test case. (I default enable BuildTimeLimit in my local, didn't meet any compiling fail test case yet.)
I think we should go further to find here " GEP's optimization is Mandatory" is make sense or not.



================
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:
> 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.


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

https://reviews.llvm.org/D129352