[PATCH] D97077: [SCEV] Pass an explicit context to computeKnownBits

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

In D97077#2575360 <https://reviews.llvm.org/D97077#2575360>, @nikic wrote:

> I don't think this is solving the problem in the right place -- this is a general `computeKnownBits()` issue, not specific to SCEV. See D93974 <https://reviews.llvm.org/D93974> for an alternative approach, and the discussion there regarding how to avoid the ephemeral value issue.

These are definitely related, but not, I think, directly the same.  I'd actually started with an approach very similar to that review before switching course.

Right now, I see three related issues: 1) arguments should use the first instruction in the function as context, 2) non-instruction values (such as globals!) should use the first instruction in the function as context, and 3) we should not bail out on an assume which is the context instruction.

The linked patch tries to target (1) and (3).  This patch specifically only targets (1) and (2).  The choice not to include (3) was very deliberate to allow step wise progress.  We do need a generic solution to (3), but it shouldn't block (1) or (2).

I'll also note that (2) can not (in general) be done inside computeKnownBits.  Why?  Because we don't know the function context it's invoked in.

While writing the above, it occurred to me that I don't directly test (2) (i.e. non-argument non-instructions).  I will add that shortly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97077