[llvm] [DSE] Fixes bug caused by skipped read clobbers (PR #83181)

[Florian Hahn via llvm-commits]

fhahn wrote:

DSE

> > It is the removed dead store (at %struct.byte.4) that invalidates the batchAA right?
> 
> Yes.

This is an interesting case, thanks for sharing! AFAIK `BatchAA` caches pairs of memory locations; removing the store itself shouldn't invalidate anything in the cache. 

I noticed that the `memset` doesn't get removed incorrectly deterministically; sometimes it gets removed, sometimes it does not, so I took a closer look at whats' going on.

When removing a store, we also remove any instructions that become dead after removal (reversibly), so we may remove an instruction that was used as memory location pointer in the cache. In most cases, this isn't a problem either, as the instruction has been removed and we are left with stale entries in the cache, which shouldn't be accessed though.

What seems to be happening in the case at hand is that we create a new GEP for the adjusted memset and in some cases it gets allocated at the same address as a previous GEP that we removed, but is still in the cache.

I experimented a bit to confirm this was indeed the issue and it looks like we should be able to keep using BatchAA, if we avoid deleting instructions that may be cached before the end of DSE, which I sketched here https://github.com/llvm/llvm-project/pull/83411

https://github.com/llvm/llvm-project/pull/83181