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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

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

> Despite the fact that this takes an "Instruction*", it's not really trying to compute some property of users of that instruction; it's trying compute some property of users of the equivalent SCEV expression.  So any computation based on the position of the instruction in the loop is not going to give the result you want: even if users of the given instruction can be assumed to be never poison, you can't generalize that property to all equivalent values in the loop.


Thanks Eli! I missed that the instruction here is indeed not suitable for position based inference. It seems a bit unfortunate to me that we fail to make use of that information, until we actually construct a equivalent expression for which it does not hold :( Not sure how worthwhile/feasible it would be to try to invalidate the information. Do you know if something like that was explored previously?

> 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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79746