[PATCH] D97092: [ValueTracking] Handle assumes on arguments with context instruction

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 22 09:14:49 PST 2021


reames added a comment.

In D97092#2576506 <https://reviews.llvm.org/D97092#2576506>, @nikic wrote:

> And I think you're right that we should avoid this. That does mean that all four patches are ultimately broken (heh).

Unfortunately, I think you're right.  I spent way more time this weekend trying to find a principled work around, and nothing I've found so far works.  I'm not quite ready to admit defeat here, but this does appear to be a hard to bypass problem.

One point worth highlighting is that the bad thing we're trying to avoid is a loss of the information encoded in an assume.  It's not a correctness issue.  From a purely pragmatic perspective, we may wish to explore whether the benefit of increased inference is worth the information loss.  I suspect that tradeoff is different for each patch, and I have not really explored in that direction yet.  It's also worth noting that the current mechanism does allow some assumes to be removed, so we wouldn't be adding that behavior.

> For SCEV, the right approach would probably be to do something similar to LVI, and take assumptions into account not directly on the value itself, but on each use, i.e. `assume(x >= 0); x >> 1` would apply the assume only when computing the range of `x >> 1`.

Unfortunately, this would be a fairly major redesign of SCEV.  I don't think this is a practical path forward.  The main problem is that SCEV doesn't track an explicit use list for SCEVExprs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97092



More information about the llvm-commits mailing list