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

[Florian Hahn via Phabricator via llvm-commits]

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.


AFAICT, there are at least 2 issues that need addressing if we want to handle LCSSA PHIs differently: 1) teach SCEV to look through LCSSA phis and 2) updating SCEV expander to insert LCSSA Phi nodes, if required. Incidentally, I started out with a patch to fix the issue in the expander. But addressing both 1) and 2) seems like a more fundamental change.

As a first step I guess we could try to find some cases where we would get more precise results by looking through LCSSA PHIs. IMO that would be a better motivation than just the expander issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68194