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

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

Eli,

Sorry for the silence.  Partly busy, and partly having a hard time explaining something which seems obvious to me.  Let me give this another try.

First, let's start with the interface question.  You seem to be assuming that the only time we'll query facts about two SCEVs is when they're both trip counts.  This is blatantly untrue.  We will frequently query relations between SCEVs when e.g. discharging loop exits in IndVarSimplify.  (And a bunch of other places.)  Having those queries return unknown more often would be undesirable.

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.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103656