[PATCH] D71539: [SCEV] Look through trivial PHIs.

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added a comment.

Some of the tests added in `llvm/test/Analysis/ScalarEvolution/trivial-phis.ll` come from https://reviews.llvm.org/D71492 but don't have the same expected results. @fhahn do you plan to improve those in this patch or a subsequent patch, or were you planning to leave those to me in D71492 <https://reviews.llvm.org/D71492>?



================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:3721
+          (!isa<PHINode>(V) ||
+           SimplifyInstruction(cast<PHINode>(V),
+                               {getDataLayout(), &TLI, &DT, &AC}) == V))
----------------
`SimplifyInstruction(V,..)` will never return `V`, so here we are saying do not insert into the map if we have a phi! Did you mean 

```
(!is<PHINode>(V) || SimplifyInstruction(cast<PHINode>(V), {getDataLayout(), &TLI, &DT, &AC}) != nullptr))
```
meaning if "we have a phi that cannot be simplified" ?


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:6562
+        if ((!isa<PHINode>(I) ||
+             cast<PHINode>(I)->getNumIncomingValues() == 1) ||
+            !isa<SCEVUnknown>(Old)) {
----------------
use `hasConstantValue()` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71539