[PATCH] D110060: [LoopBoundSplit] Handle the case in which exiting block is loop header

[JinGu Kang via Phabricator via llvm-commits]

jaykang10 added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopBoundSplit.cpp:404
+    } else if (SE.isSCEVable(PN.getType()) &&
+               isa<SCEVAddRecExpr>(SE.getSCEV(&PN))) {
+      // Update phi which is not related to exit condition but its expression is
----------------
mkazantsev wrote:
> This is fundamentally wrong for two reasons.
> 
> First - we should never base our logic on SCEV's ability to prove some fact, even if we know this for sure and SCEV was able to prove the very same thing in the past.
> 
> In particular - we should not even rely on (quite obvious) assumption that SCEV will always be able to prove that something that is increases by a constant has AddRecSCEV. It is likely to happen, but SCEV has *no guarantees* that it will surely prove things.
> 
> Second - and what about phis that are not addrecs? Do they not need update? For example, imagine code
> ```
> for (i = 2; i < N; i = i * i) {...}
> ```
> The phi here won't be an addrec. But it will still need an update when you change the iteration space, or at least I don't see any reason why it won't.
You are right. Let me think about how to handle the phis.


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

https://reviews.llvm.org/D110060