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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:3721
+          (!isa<PHINode>(V) ||
+           SimplifyInstruction(cast<PHINode>(V),
+                               {getDataLayout(), &TLI, &DT, &AC}) == V))
----------------
bmahjour wrote:
> `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" ?
I think this check got messed with during a previous rebase a while back or covered a problem with earlier versions of the depending patches. This special case should not be necessary with the latest version, none of the tests that I added for this work earlier are impacted and I removed it.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:6562
+        if ((!isa<PHINode>(I) ||
+             cast<PHINode>(I)->getNumIncomingValues() == 1) ||
+            !isa<SCEVUnknown>(Old)) {
----------------
bmahjour wrote:
> use `hasConstantValue()` instead?
This special case should not be necessary with the latest version, none of the tests that I added for this work earlier are impacted and I removed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71539