[PATCH] D104013: [LangRef] State that the based-on relation is for pointer typed values only

[Harald van Dijk via Phabricator via llvm-commits]

hvdijk added inline comments.


================
Comment at: llvm/docs/LangRef.rst:2703
+stored in the memory by loading it as a non-pointer type loses its associated
+address range.
+
----------------
nhaehnle wrote:
> hvdijk wrote:
> > This implies that storing a pointer value in memory and loading it as a pointer type does preserve its address range. Whether it does is unclear; this idea is assumed by some existing optimisations and incompatible with other existing optimisations. In my opinion, it is not a good idea to update the documentation to clarify this either way until it is decided how LLVM should behave.
> > 
> > If you have raised this not to update the documentation directly, but only to as a starting point for discussion, could you also include what happens when an integer is stored to memory and is then loaded as a pointer value?
> > This implies that storing a pointer value in memory and loading it as a pointer type does preserve its address range. Whether it does is unclear; this idea is assumed by some existing optimisations and incompatible with other existing optimisations
> 
> If store of pointer followed by load of pointer *doesn't* preserve the address range, then mem2reg is likely incorrect as-is, so... :)
> 
> Which existing optimizations are incompatible with this?
Consider
```
define i8* @f(i8* %p) {
  %buf = alloca i8*
  %buf.i32 = bitcast i8** %buf to i32*
  store i8* %p, i8** %buf
  %i = load i32, i32* %buf.i32
  store i32 %i, i32* %buf.i32
  %q = load i8*, i8** %buf
  ret i8* %q
}
```
I think it is clear that this needs to return `inttoptr(ptrtoint(%p))`, SROA optimizes it to exactly that, but Early CSE then changes it to return `%p` (tested in LLVM/clang 12.0.0). That looks wrong. Let's suppose it didn't do that, let's suppose we run existing optimisations in a different order. Given in the above
```
  %i = load i32, i32* %buf.i32
  store i32 %i, i32* %buf.i32
```
Early CSE is able to remove this store by looking at only these two instructions without any other context: the store is a store of exactly the value that was just loaded. If storing a pointer value in memory and loading it as a pointer type preserves the address range, though, this is invalid: the previous store to that address was a pointer with provenance, and this store is an integer without provenance, so the store still had an effect. This optimisation is only correct if pointers in memory lose their address ranges.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104013