[PATCH] D102615: [LoopDeletion] Break backedge if we can prove that the loop is exited on 1st iteration (try 3)

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev added a comment.

Thanks Nikita, I think I have some arguments for this philosophical debate. :)

> This patch falls into the very unfortunate category where a) it solves some specific problem you have, b) outside that, it applies very rarely

I'd argue that. Just from how many failures we've seen when irreducible CFG wasn't handled correctly, I conclude that there have been many attempts to make this transform even in these weird cases. Provided how rare those CFGs are, I can speculate that there have also been many cases when it did apply on regular CFG too. Even in the test suite, there were 2 loops of this kind, and I'd expect it to not have such patterns at all.

Either way, "rare or frequent" is a very speculative category. You can say "meh, it's just 2 of them in the test suite" or "wow, it's 2 of them somewhere I didn't expect this!" For us, this pattern is coming from a very common abstract I/O algorithm, which has logic of "read until you've read enough", and for some of its implementations it knows that it will always read whole lot at once. I'd not expect the test suite to be a place to heavily exercise things like this, and if it was found there - cool.

As for SCEV impact, some points.

1. Yes, I agree that we should try to reduce impact on compile time when we can;
2. We already have passes (IV simplify, IRCE, LSR...) where we already use SCEV. I'm absoultely certain that in reality, most things they do they could handle with InstSimplify-like instructions. But for some reason, we don't mind against the toll that SCEV takes there (and it should be HUGE in IV simplify) just because we started tracking compile time *after* these things were already there.
3. It's pretty clear to me that the problem is not in loop deletion that is slow with SCEV. The problem is that the SCEV is slow. And it's much slower than 0.2% that we see here. Holding away future optimizations just because SCEV is, and always has been, slow, doesn't seem right to me.

Yes, we can in theory simplify this thing to InstSimplify. We save 0.2% CT. And we still keep wasting tons of CT in IV Simplify for example (I don't know how much, but pretty sure it's well above 10%).

I think we all agree that slowness here is just from SCEV, not from the change I made. So my suggestion is that we should be fighting the root cause, which is SCEV inefficiency or slowness, and not try to fight accidental manifestations of this inefficiency. I can commit to spend some time deliberately looking for places where we can save time in SCEV.


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

https://reviews.llvm.org/D102615