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

[Eli Friedman via Phabricator via llvm-commits]

efriedma created this revision.
efriedma added reviewers: nikic, fhahn, mkazantsev, reames, lebedev.ri.
Herald added subscribers: bmahjour, javed.absar, zzheng, kbarton, hiraditya, nemanjai.
efriedma requested review of this revision.
Herald added a project: LLVM.

Rules for determining if a SCEV expression is pointer-typed:

1. SCEVUnknown is a pointer if and only if the LLVM IR value is a

pointer.

2. SCEVPtrToInt is never a pointer.
3. If any other SCEV expression has no pointer operands, the result is

an integer.

4. If a SCEVAddExpr has exactly one pointer operand, the result is a

pointer.

5. If a SCEVAddRecExpr's first operand is a pointer, and it has no other

pointer operands, the result is a pointer.

6. If a SCEVMinMaxExpr has all pointer operands, the result is a

pointer.

7. Otherwise, the SCEV expression is invalid.

I'm not sure how useful rule 6 is in practice.  If we exclude it, we can
guarantee that ScalarEvolution::getPointerBase always returns a
SCEVUnknown, which might be a helpful property. Anyway, I'll leave that
for a followup.

Currently has some regression test failures related to LSR; the
generated code is changing.  scev-custom-dl.ll currently crashes because
getPointerDiff() can return SCEVCouldNotCompute for exotic pointers.
Otherwise passes regression tests.

Open questions:

1. Is it worth keeping getPointerDiff separate from getMinusSCEV?  It's

an extra SCEV entry point which is in some sense redundant.  But one
advantage is that the new API lets us land the changes to various
transforms incrementally.

2. How do we want to deal with ScalarEvolution::isImpliedCond?

Currently, I have a few targeted bailouts for pointers, but we could
just ptrtoint all the operands.

3. What do we do about exotic pointers?  None of the places that use

getPointerDiff have any handling for failure.

4. What do we do about LSR?  CollectFixupsAndInitialFormulae()

subtracts two pointers to model an icmp; using getPointerDiff here is
correct, but wrecks a bunch of the heuristics because "ptrtoint %x" is
not the same SCEV expression as "%x". And LSR is really fragile;
touching almost anything to try to address that will affect other tests.
Not sure what to do here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104498

Files:
  llvm/include/llvm/Analysis/ScalarEvolution.h
  llvm/lib/Analysis/Delinearization.cpp
  llvm/lib/Analysis/DependenceAnalysis.cpp
  llvm/lib/Analysis/LoopAccessAnalysis.cpp
  llvm/lib/Analysis/LoopCacheAnalysis.cpp
  llvm/lib/Analysis/LoopUnrollAnalyzer.cpp
  llvm/lib/Analysis/ScalarEvolution.cpp
  llvm/lib/Analysis/ScalarEvolutionAliasAnalysis.cpp
  llvm/lib/Analysis/StackSafetyAnalysis.cpp
  llvm/lib/Analysis/VectorUtils.cpp
  llvm/lib/Target/PowerPC/PPCLoopInstrFormPrep.cpp
  llvm/lib/Transforms/Scalar/AlignmentFromAssumptions.cpp
  llvm/lib/Transforms/Scalar/LoopDataPrefetch.cpp
  llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp
  llvm/lib/Transforms/Scalar/LoopRerollPass.cpp
  llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp
  llvm/lib/Transforms/Utils/ScalarEvolutionExpander.cpp
  llvm/lib/Transforms/Vectorize/LoadStoreVectorizer.cpp
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
  llvm/unittests/Analysis/ScalarEvolutionTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D104498.352884.patch
Type: text/x-patch
Size: 29520 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210618/030ab118/attachment.bin>