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

Thu Jun 17 01:56:52 PDT 2021

mkazantsev added a comment.

@reames

> One thing I'm wondering about is whether we're invalidating SCEV too aggressively in one of the existing transforms. I don't have strong evidence of this, but computing SCEVs for a loop isn't *that* expensive, and given a properly structured loop pipeline should happen approximately once. I'd be tempted to instrument the various "forgetX" calls and run one of the worst offenders compile time wise. Do all of the invalidation make sense? Just based on a quick skim of the code, I see a couple call sites in LoopSimplifyCFG and IndVarSimplify which look likely to invalidate far more than is required. In particular, if we were invalidating multiple times in a loop nest, that could cause degenerate compile time.

Even if so, it's not a problem of this patch, right? I can spend some time trying to investigate this if we all have agreement that it can go with SCEV if we can win some time elsewhere. We don't have this agreement so far. 2 concerns I have:

1. I don't believe that 0.2% time is something that can be reliably measured by any tool available to me, even if I can somehow get the environment to measure clang;
2. SCEV invalidation would be the last place to find this "elsewhere", because bugs caused by un-invalidated SCEV are *extremely* (5 times underlined) nasty. I don't want to commit to investigate and fix all of them, unless tortured, or hung above a volcano. :)

> It's hard to tell whether there's another approach here without seeing the original motivating example. The fact that all of the test cases are handled by SimplifyInst (which, btw, unrolling already has most of the code for) raises questions here. It'd be nice to put those to bed once and for all.

Currently scheming a prototype to see if it works. My speculation is InstSimplify might be enough for my motivating example. However, this will require massive refactoring of InstSimplify itself (some of its static functions will need to be made publicly available). It's a lot of work just to duplicate a part of what SCEV does out of box.

@nikic

> The case here is different: This optimization only has a nominal relation to loops.

I disagree. If we think something is a loop, we will run all our loop passes (including IndVars and LSR) on it. However, if loop deletion proves it's not a loop, we may save time on costly opts in other places. But again, whether this will or will not happen frequently, or whether the overall impact is positive, is a very speculative ground on which neither of us have (or may have) enough evidence.

> There are no add recurrences involved, there is no "scalar evolution" going on -- rather, SCEV is being used as a generic symbolic reasoning framework, which is not a use-case it is intended or optimized for.

And LLVM is not "low level" and not a "virtual machine", right? :) It's just a wordplay.
Just because the initial motivation for SCEV was loop passes (maybe IndVars), I don't see any reasons why it cannot be used as generic symbolic framework. It has all power of it, it has tons of simplification that has nothing to do with loops, its implication engine does a lot to prove non-recurrent things. Today we even use it for pointers. I see no reason why we should stick to something that was relevant in the past and hold away the further expansion of its usage.

> Of course, there is the little caveat that we don't have any other symbolic reasoning framework in LLVM that has the same level of power, so I can certainly appreciate why it is being used here. I just want to highlight that this is not a case where SCEV is the obviously correct choice, as it is for passes like IndVars or LSR.

I think that we should not have multiple framework that do the same thing, some of them being more and some being less powerful. What you are saying makes sense, but to me it just means that we need a way to make SCEV more lightweight. Maybe configure its prove engine to limit to the simplest ways of proving thigs.

I'll give a chance to a version that only uses the simplest way of proving thigs through non-recursive reasoning, and try to wright a version with InstSimplify as PoC.

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

https://reviews.llvm.org/D102615