[PATCH] D90066: [BasicAA] Fix caching in the presence of phi cycles

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

Thank you for this patch, I think this is a better general direction.

1. Did you happen to test the compile-time impact for this? (it's a correctness fix, and needed, but it would be good to note on what to expect)

2. I think the IsCapturedCache cache can be reused in the new AAQI. Ideally the new AAQI would use a reference to it, not a copy, which would need a bit of a re-write but may be worth as far as performance.

3. Could you add to the test something like this (this was defeating my patch):

  BatchAAResults BatchAA2(AA);
  EXPECT_EQ(NoAlias, BatchAA2.alias(A1Loc, A2Loc));
  EXPECT_EQ(MayAlias, BatchAA2.alias(S1Loc, S2Loc));
  EXPECT_EQ(MayAlias, BatchAA2.alias(PhiLoc, A1Loc));

Looking further to see if there's anything I missed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90066