[PATCH] D116766: [SCEV] Sequential/in-order `UMin` expression

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

In D116766#3231853 <https://reviews.llvm.org/D116766#3231853>, @nikic wrote:

> In D116766#3231777 <https://reviews.llvm.org/D116766#3231777>, @reames wrote:
>
>> I skimmed the code for a sanity check, but am mostly just commenting on the high level bits.
>>
>> I'm not convinced this is the right approach, but I also don't want to block iterative progress.  My primary concern is that I don't see how this solves the multiple exit case and that I think we're going to end up needing a new representation there, which indirectly solves this problem.  However, I think we can work step-wise here, and come back to refactor/clean this up if needed.
>
> Why do you think this would't solve the multiple exit case? The new node is currently not used for that case, but I do believe it would address that case as well (if used).

The case I was thinking of was when exit1 was taken on iteration N, but exit2's condition becomes poison on that same iteration.  However, thinking about it harder, I think the existing code is required to simply return N+1 for exit2, and that's not directly a problem.  However, by that logic we don't have a problem around multiple exits at all, so I'm clearly missing/forgetting something.

So, maybe it does?  I haven't fully thought through that problem yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116766