[PATCH] D73181: [SCEV] Do not use backedge SCEV of PHI if its input is another PHI

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 23 02:36:04 PST 2020


mkazantsev added a comment.

LGTM with nit (see inline).



================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:8246
+            const SCEV *OnBackedge = getSCEV(BackedgeVal);
+            if (!isa<PHINode>(BackedgeVal) &&
+                IsAvailableOnEntry(LI, DT, OnBackedge, PN->getParent()))
----------------
dantrushin wrote:
> mkazantsev wrote:
> > Your check is overprotective. If `BackedgeVal` is a Phi from another loop than PN's (e.g. outer loop or some other loop going before it), it is OK to use it as exit value. The problem only happens if `PN->getLoop() == BackedgeVal->getLoop()`, am I correct?
> You're right.
> I've been cutting edges to keep code cleaner ;)
Looks correct now, but I'd rather ask you to factor out this check into a lambda (smth like `IsPhiFromSameBlock`); makes it easier to understand what happens.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73181





More information about the llvm-commits mailing list