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

Mon Oct 24 12:50:57 PDT 2022

jdoerfert added a comment.

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?

================
Comment at: llvm/test/Transforms/Attributor/value-simplify-pointer-info.ll:784
 ; TUNIT-NEXT:    [[I3:%.*]] = getelementptr inbounds [[STRUCT_S]], %struct.S* [[AGG_RESULT]], i64 0, i32 2
-; TUNIT-NEXT:    store i32 4, i32* [[I3]], align 4, !tbaa [[TBAA14]]
+; TUNIT-NEXT:    store i32 [[ADD2]], i32* [[I3]], align 4, !tbaa [[TBAA14]]
 ; TUNIT-NEXT:    ret void
----------------
Why do we miss out on these propagations? There are no PHIs involved, right? Something is amiss.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136526