[PATCH] D103656: [ScalarEvolution] Ensure backedge-taken counts are not pointers.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

> Having those queries return unknown more often would be undesirable.

I agree.

> Second, the getLosslessPtrToIntExpr routine seems to not match what you're describing. It appears to have a bunch of cases where it does not succeed. Examples include min/max expressions, and udivs. (Going by code structure here. If I'm missing something, let me know.) Are you claiming that this routine is guaranteed to succeed? The existing code and comments don't seem to make this obvious.

SCEVPtrToIntSinkingRewriter always succeeds; it inherits implementations for every kind of expression from SCEVRewriteVisitor.  The explicit implementations of visitAddExpr and visitMulExpr are just there to preserve the flags.  Not sure how you're reaching the conclusion that it fails for min/max expressions.

> If you want to achieve the stated goal, have you tried checking for lossless conversion of the *result* of the exit computation? If that worked, it would be a much cleaner and more obvious change. (e.g. never return an exit count which can't be converted to integer type) This would mean instrumenting the construction of ExitLimit to drop information if needed. Or maybe that part of your change with an assert in the ExitLimit constructor? Not sure there.

This could work as an intermediate step.  Digging a little, it wouldn't be that painful; there aren't that many places in howFarToZero and howManyLessThans/howManyGreaterThans that would need changes.

But we eventually want something like the current patch, I think.  We don't really want to be doing computations like `getMinusSCEV(LHS, RHS)` where LHS and RHS are pointers. Currently, that returns a SCEV with pointer type, but it doesn't really make sense to treat it as a pointer, particularly in the context of an "icmp eq".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103656