[PATCH] D50985: [SCEV] LoopsUsed memoization

[Max Kazantsev via Phabricator via llvm-commits]

mkazantsev requested changes to this revision.
mkazantsev added a comment.
This revision now requires changes to proceed.

I have a lot of doubts that it is correct. It might be, but I would request you to add some tests which aggressively exercise scenarios like "cached something -- called forgetMemoizedResults/forgetLoop -- attempted to get cached results". I am specifically worried about CFG changes and loop deletions.



================
Comment at: lib/Analysis/ScalarEvolution.cpp:11763
   MinTrailingZerosCache.erase(S);
+  LoopsRefd.erase(S);
 
----------------
I don't think it is sufficient for correctness. Imagine the situation: `A = {1,+,1}<some_loop>, B = A + 1`. Something has changed for A, and it is no longer using `some_loop`. How do we say that `B` is not using it as well?

I also think that something needs to be done about it in `forgetLoop`. This method is called, for example, when a loop becomes non-existent, or when the set of its blocks changes. And you may end up with many cache bundles keeping references to this loop.


Repository:
  rL LLVM

https://reviews.llvm.org/D50985