[PATCH] D44748: Track whether the size of a MemoryLocation is precise

[George Burgess IV via Phabricator via llvm-commits]

george.burgess.iv created this revision.
george.burgess.iv added reviewers: hfinkel, reames, nlopes.
Herald added subscribers: david2050, sanjoy.

In the vast majority of cases, we hand AA precise sizes for `MemoryLocation`s, rather than upper-bounds. BasicAA has a few checks that assume the sizes passed in are precise, which breaks down when we start passing in upper-bounds that might include conditionally-executed UB (which is what AliasSetTracker is doing right now, and which caused PR36228). The most straightforward way to fix this without penalizing AA seems to be passing an extra "I guarantee you this size is precise" bit along.

Passing this bit in a minimally invasive way appears difficult in pathological cases (read: massive sizes), and we're already sort of treating this Size as an `Optional<uint64_t>`, so I figured I'd wrap this up in its own type to make the properties of MemoryLocation sizes more obvious/hopefully easier to deal with.

As a natural part of this refactoring, PR36228 gets fixed.

If this lands, I will do the following cleanups (...which there's a lot of, but the majority are trivial):

- Replace all uses of `MemoryLocation::UnknownSize` with `LocationSize::unknown()` + remove MemoryLocation::UnknownSize
- Replace the various resulting `Loc == LocationSize::unknown()`s with the shorter, equivalent `!Loc.hasValue()`
- Migrate users of the implicit `LocationSize(uint64_t)` ctor to `LocationSize::precise` or `LocationSize::upperBound`
- Remove the `LocationSize(uint64_t)` ctor, which will force callers of AA APIs to reason about/be explicit about the precision of the `LocationSize` they're passing.

The last two bullets are the bits that I hope will prevent future bugs like this (and catch other existing ones, if any). :)

Any thoughts/comments welcome, as always.


https://reviews.llvm.org/D44748

Files:
  include/llvm/Analysis/AliasAnalysis.h
  include/llvm/Analysis/AliasSetTracker.h
  include/llvm/Analysis/BasicAliasAnalysis.h
  include/llvm/Analysis/MemoryDependenceAnalysis.h
  include/llvm/Analysis/MemoryLocation.h
  lib/Analysis/AliasSetTracker.cpp
  lib/Analysis/BasicAliasAnalysis.cpp
  lib/Analysis/CFLAndersAliasAnalysis.cpp
  lib/Analysis/MemoryDependenceAnalysis.cpp
  lib/Analysis/ScalarEvolutionAliasAnalysis.cpp
  lib/Target/Hexagon/HexagonLoopIdiomRecognition.cpp
  lib/Transforms/Scalar/DeadStoreElimination.cpp
  test/Transforms/LICM/pr36228.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44748.139337.patch
Type: text/x-patch
Size: 51068 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180321/ffdc37b8/attachment.bin>