[PATCH] D109958: [LoopFlatten] Enable it by default

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

In D109958#3862780 <https://reviews.llvm.org/D109958#3862780>, @nikic wrote:

> I looked into the unexpectedly large compile-time regressions this pass causes, and this does appear to come down to the move from LPM2 to LPM1. I believe the reason why this has such an impact is this: Prior to this patch, passes in LPM1 did not use SCEV, only passes in LPM2 did. The move of LoopFlatten introduced a SCEV use in LPM1, which would get computed just for LoopFlatten, and then discarded again. (Note that all loop passes depend on SCEV, but it's a lazy analysis, so what matters is whether SCEV actually gets queried.)

Hm, I've just recommitted this as the sanitizer bot failures seemed unrelated.

> The fact that LoopFlatten does not work properly after IndVarSimplify is fairly concerning, because IndVars is an IV canonicalization pass, and other passes are supposed to deal with the IVs it produces. The fact that LoopFlatten actually does its own IV widening (which is generally the responsibility of IndVars) is further indication that something is going wrong here.

Not sure I fully agree with "does not work properly", more precise would be to say less effective. Less effective because there's less chance of it triggering. The IV widening helps or rather avoids overflow checks. If IndVars comes along first, that opportunity is gone. With the widened IVs, the legality is very difficult to determine. That's the rationale and design decision of this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109958