[PATCH] D104498: [WIP][ScalarEvolution] Strictly enforce pointer/int type rules.

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

Spent some further time prototyping this.   I mocked up a quick and dirty patch which simply made all calls to getMinusSCEV with two pointer arguments return CouldNotCompute.  There were surprisingly few places which needed adjusted both based on failing tests, and some (quick) manual skimming.   The end result is something which mostly doesn't crash on make check, but bails out of every pointer subtraction query.

Unfortunately, there's some *major* optimization impact.  This is to be expected, but given it's the baseline which is being considered for non-integral pointers, we need to assess whether the impact is reasonable.

A few key points:

1. We completely break dependency analysis as used by the vectorizer, fusion, and distribution passes.
2. We weaken LSRs ability to fold together related expressions.
3. We completely break SCEV-AA.

(1) is somewhat of a non-starter.  Before we get to how to craw that back, a digression...

Thinking through what the existing code does for subtraction of non-integral pointers, I'm not sure it's even correct for the general case.  When we can find a common base in the expressions, everything works out to an integer result, but in the case we can't, we seem (based on the code) to either crash or generate the ptrtoint after all.  I'm going to try to find a test case for this, I haven't managed yet.

Given all of that, I think this is the right approach, but we're going to have to deal with the opt impact.  One approach is to add specific folding rules for when the base pointer for the two expressions is the same.  I think we can recover most of the impact, and suggest using the proxy of "how many test cases fail if I treat *all* pointers this way* as a reasonable metric.  I expect we can drive this "close enough" to zero without too much effort.

Style wise, I really don't see the value in the getPointerDiff api.  Most of the places which need to compute a diff work with both pointers and integers.  I'd rather just have getMinusSCEV documented as being able to return CouldNotCompute when both operands are pointers.

I also want to preserve the existing "it's an error to query information about CouldNotCompute" structure.  Given this, callers should bail before calling isKnownX and friends.  We should strengthen asserts on access.  A couple of the stack traces got surprisingly far into SCEV before we hit an access assert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104498