[PATCH] D79746: [SCEV] Relax abnormal exit check in isAddRecNeverPoison.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

In D79746#2032220 <https://reviews.llvm.org/D79746#2032220>, @fhahn wrote:

> In D79746#2030380 <https://reviews.llvm.org/D79746#2030380>, @efriedma wrote:
>
> > I think there might be another way to handle the testcase, though.  If the input instruction is a PHI node, we can use PHI-specific reasoning.  In this context, we don't care if the base value is poison, I think; we're trying to compute a property of the increment.  And the "add nsw" can't be poison because it's also used by the loop latch of the previous iteration: if it's poison, we can't reach the current iteration, regardless of whether the loop has other exits.
>
>
> Sounds good! I also explored something similar earlier, but somehow thought the current version of the patch seemed a bit more direct (but I missed the bit about not being able to use the location here). I'll update the patch tomorrow.


Hm I think the reasoning does not really help here unfortunately, as `isAddRecNeverPoison` takes the increment instruction of the AddRec I think .

IIUC the problematic case is if the have an `%iv.next = add nsw %iv, 1`, we know that %iv cannot be poison (and %iv.next from the previous iteration also cannot be poison). But `%iv.next` could overflow in the current iteration and then take an abnormal exit, hence we cannot assume the AddRec never overflows.

In D79746#2032381 <https://reviews.llvm.org/D79746#2032381>, @efriedma wrote:

> On a more general topic, there's been some interest in changing the way poison flags on SCEV expressions work, so it's possible to have both a poison and non-poison SCEV computing the same value.  But nobody has actually tried to implement it, I think.


Would the idea here be allowing to use overflow flags for expressions depending on where an expression is evaluated/used? (for example the latch branch for the trip count computation)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79746