More tests are added.

In D67207#1660225, @brzycki wrote:

Hello @skatkov , I'm sorry to hear you're dealing with an intermittent crash. If you find a reproducible .ll file you can feed into opt -jump-threading -S < input.ll that crashes I would be interested in examining it. The Deferred usage of DTU causes unexpected issues and it is entirely possible there is still a bug here.

(CC'ing @kuhar and @NutshellySima to keep them informed)

In D67359#1664242, @wmi wrote:

In D67359#1664241, @skatkov wrote:

I wonder whether the problem is in this function or in analysis itself?
If it is a problem of analysis it is better to restrict it but not an utility function.

I feel the problem is in the utility function. Currently the problem is exposed in IVUser pass but the same can happen anywhere else if only the function is called for a huge loop with many exits and with each exit having many predecessors.

I wonder whether the problem is in this function or in analysis itself?
If it is a problem of analysis it is better to restrict it but not an utility function.

Thank you guys, for feedback.

Note: I've got the crash by changing the DomTreeUpdater form Lazy mode to Eager. Even with this fix it will not work because of LVI->eraseBlock(&BB);
It seems the code is written in the way DTU cannot be Eager.

Please provide a quick fix or revert the patch causing the crash due to transforming to incorrect IR.

Please provide a quick fix or revert the patch causing the crash due to transforming to incorrect IR.

Abandon in favor of https://reviews.llvm.org/D64972

Abandon in favor of https://reviews.llvm.org/D64972

In D64972#1607998, @skatkov wrote:

In D64972#1607909, @skatkov wrote:

Philip, please take a look at https://reviews.llvm.org/D65501 which introduces the peeling level as possible solution to handle your comment.

See https://reviews.llvm.org/D65503. It is the same patch but based on D65501.

In D64972#1607909, @skatkov wrote:

Philip, please take a look at https://reviews.llvm.org/D65501 which introduces the peeling level as possible solution to handle your comment.

Philip, please take a look at https://reviews.llvm.org/D65501 which introduces the peeling level as possible solution to handle your comment.

In D64972#1606836, @reames wrote:

Having both the enable flag and the AlreadyPeeled variable is really confusing. Is there a way we could combine them? Maybe replace the boolean with the AlreadyPeeledViaProfiling count or something?

Added the guard to not exceed the UnrollPeelMaxCount limit.

the variable and option name changed.

Test is updated to show what this patch changes.

In D64972#1593826, @reames wrote:

I'm not quite sure this is the right macro approach.

The comment near the code your modifying makes me thing the whole reasoning behind disabling future peeling and unrolling after the first peel may be flawed. It doesn't make sense to "use up" profiling information. Assuming we correctly updated our profiling when doing the transform, the resulting profile for the loop should indicate that it's cold and thus not profitable to further peel/unroll. If that's not happening, maybe there's another issue in play? (I wonder if your other profile bug fix may help here?)

p.s. I'm opened to being convinced that this is a practical answer, even if not an ideal one. Just make the argument. :)

Please, take a look.

Hi Philip, Thank you for review. I'll update a patch.

Abandon in a favor of https://reviews.llvm.org/D64235

Thanks to @davezarzycki who fixed it in
commit 12400b97838d78386920cbda3c9dadb9f8727ce3
Author: David Zarzycki <dave@znu.io>
Date: Mon Jul 15 14:12:35 2019 +0000

I decided to avoid using public API with predicate due to it will require introducing template function in template class and effectively moving implementation to LoopInfo.h

Make getUniqueExitBlocksHelper more general with using filter predicate.
Unit test is added.

In D64553#1580189, @lebedev.ri wrote:

May be good to have a test.