[PATCH] D136526: [AAPointerInfo] refactor how offsets and Access objects are tracked

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds added a comment.

In D136526#3877140 <https://reviews.llvm.org/D136526#3877140>, @jdoerfert wrote:

> Can you point out the test that this fixes? Maybe a standalone reproducer would be good too. I read only the commit message and I'm not sure I understand the problem.

I did leave comments on the Phab page for that other commit ... https://reviews.llvm.org/rGad98ef8be409 . But I guess it's incorrect to say that the other commit introduced the bug. It's a latent bug that was exposed by tests like phi_no_store_2.

Essentially, the PHI %p in phi_no_store_2 is visited twice. The first time, it is assigned the offset 0, and the second time it is set to unknown. This results in the following bins being printed in the debug log:

Accesses by bin after update:
[0-1] : 1

- 10 -   store i8 1, i8* %p, align 1
  - c: i8 1

[-2--1] : 1

- 10 -   store i8 1, i8* %p, align 1
  - c: i8 1

Then forallInterferingAccesses comes along and sees only the first bin with the assumption that one access can only be in one bin. Hence it concludes that the store does not have a potential copy in load %l21, and hence eliminates the store. This is incorrect, since the load is still there and it should see the value written by that store. The store can be eliminated only by recognizing the potential copy and first propagating the value to the load.

The root cause is that if an access is already in a bin, and its offset is updated to unknown, then the access needs to first be removed from its current bin. This change ensures this behaviour by improving the plumbing of how accesses are tracked in bin, so that clients don't have to worry about these things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136526