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

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds added a comment.

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

> In D136526#3877410 <https://reviews.llvm.org/D136526#3877410>, @sameerds wrote:
>
>> 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.
>
> I see. The error description makes sense. Can we split this into a fix and the rewrite though? The fix would be to remove the entry from the bin as soon as the offset changes (which can only become invalid I assume). The rewrite gets rid of the bins and keeps lists of instructions per access, right? There is a change in one of the tests that I think is caused by the rewrite and should not be. We also need tests to capture the benefits of the rewrite then. Also, are the bins itself bad or simply bad that we have on offset per access?

It's the one-offset-per-access that causes loss of information. I've updated the change description to talk more about this. Hope that clarifies the situation! Also fixed the side effect on other places. I had failed to distinguish between multiple local insts pointing to the same remote inst.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136526