[PATCH] D28743: [PM] Teach the LoopPassManager to automatically canonicalize loops by runnig LCSSA over them prior to running the loop pipeline.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc marked 3 inline comments as done.
chandlerc added inline comments.


================
Comment at: include/llvm/Transforms/Scalar/LoopPassManager.h:308-310
+      // Verify the loop structure and LCSSA form before visiting the loop.
+      L->verifyLoop();
+      L->isRecursivelyLCSSAForm(LAR.DT, LI);
----------------
davide wrote:
> sepavloff wrote:
> > davide wrote:
> > > These two can be a bit expensive. Maybe put them under `#ifndef NDEBUG` ?
> > In D28676 the same problem were solved by making `VerifyLoopInfo` global and running expensive checks only if `-verify-loop-info` is specified in command line. Maybe that change can be useful here?
> Maybe, eventually. But until all the bugs in the new PM are shaken properly my preference would be to run this check unconditionally for debug builds. (note, we do something similar for NewGVN and it revealed quite a few bugs, and these checks can be always be moved to LLVM_ENABLE_EXPENSIVE_CHECKS once we're satisfied with things and/or we think running the verification by default is crazy expensive).
Agreed on all counts. Eventually we can turn this down, but for now I'd like to be aggressive.


================
Comment at: test/Analysis/IVUsers/quadradic-exit-value.ll:8
 
-; RUN: opt < %s -analyze -iv-users | FileCheck %s
+; RUN: opt < %s -analyze -iv-users | FileCheck %s --check-prefixes=CHECK,CHECK-NO-LCSSA
 ; RUN: opt < %s -disable-output -passes='print<ivusers>' 2>&1 | FileCheck %s
----------------
davide wrote:
> Why two prefixes here?
Below, we only use the one prefix.

Without LCSSA, we get substantially more precise results here, whereas with it we get substantially more poor results. This is because of a combined weakness of SCEV and IVUsers which stems from SCEVExpander's inability to expand correctly in LCSSA form. =[ It's a mess.

I've talked to Michael (K) and Sanjoy about this and we have some ideas to fix this eventually, but not right away. (Fortunately, the only thing that cares today is LSR which is in the codegen phase and so not blocking anything.)

I wrote the FIXME above this to document what is going on here.


https://reviews.llvm.org/D28743