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

[Sanjoy Das (Work Account) via Phabricator via llvm-commits]

sanjoy.google added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:8247
+            if (!isa<PHINode>(BackedgeVal) &&
+                IsAvailableOnEntry(LI, DT, OnBackedge, PN->getParent()))
               return OnBackedge;
----------------
dantrushin wrote:
> sanjoy.google wrote:
> > dantrushin wrote:
> > > Bailing out here when any PHI is seen, because I did not find convinient API to get SCEV at previous iteration.
> > > Is there one?
> > I don't think you need to go through SCEV here.  You can just pick the `llvm::Value` for the PHI for the backedge and get the SCEV for that.
> Excuse me, I don't understand (or, rather my comment was unclear).
> 
> Existing code already does that. In terms of test case below, `PN` is PHI `%local_3_4` 
> Its backedge `Value` is `%local_6_6` which is another PHI from the same block.
> But using `%local_6_6`'s SCEV as value for `%local_3_4` is wrong, because `%local_3_4` uses value of `%local_6_6` 
> from previous iteration
> So my fix is not to perform this optimization when PHI's input is another PHI
> 
> Or you mean something else?
I was unclear as well.

I'm suggesting we do this:

```
Value *BackedgeVal = PN->getIncomingValue(InLoopPred);
if (auto* InputPN = dyn_cast<PHINode>(BackedgeVal)) {
  if (InputPN->getParent() == PN->getBlock()) {
    // PHI translation
    BackedgeVal = InputPN->getIncomingValue(InLoopPred);
  }
}
const SCEV *OnBackedge = getSCEV(BackedgeVal);
// ... no other change
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73181