[PATCH] D68194: [LCSSA] Forget values we create LCSSA phis for

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 16 04:10:36 PST 2019


fhahn added a comment.

In D68194#1698311 <https://reviews.llvm.org/D68194#1698311>, @efriedma wrote:

> > I know this contradicts existing code in SCEV that tries not to break LCSSA, I'm wondering if we should just remove those in favor of teaching SCEV expander about preserving LCSSA.
>
> Currently, SCEV deliberately doesn't look through LCSSA PHI nodes (there's code in ScalarEvolution::createNodeForPHI etc.).  You're saying we should change that?  It might be a good idea.  I recently ran into an performance issue involving a nested loop where SCEV for the outer loop was getting confused by an LCSSA PHI for the inner loop.  I'd be worried about loop passes getting confused, though; for example, if they assume all AddRecs are part of the current loop nest.
>
> That said, this patch probably isn't the right place to have that discussion.


I've put up a WIP patch that teaches SCEVExpander to preserve LCSSA while expanding: D71538 <https://reviews.llvm.org/D71538> it still needs a bit of refactoring and cleanup. And D71539 <https://reviews.llvm.org/D71539> to look through trivial PHIs which also needs a bit of work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68194





More information about the llvm-commits mailing list