[PATCH] D53786: [AliasSetTracker] Actually delete instructions from the AliasSetTracker.

[Philip Reames via Phabricator via llvm-commits]

reames added a comment.

Hm, can I ask what problem you were originally trying to solve?  From a quick search, this code is only invoked through the value handle deleted callback.  In which case, the value being deleted is the pointer itself, not the original using instruction.

This feels like possibly a solution in search of a problem?



================
Comment at: lib/Analysis/AliasSetTracker.cpp:555
+void AliasSetTracker::deleteValue(Instruction *I) {
+  auto Loc = MemoryLocation::getOrNone(I);
+  if (Loc == None)
----------------
I get what you're going for, but this is not enough.  Some instructions (calls) can map to multiple memory locations.  You're also not removing unknown instructions.  


================
Comment at: lib/Transforms/Scalar/LICM.cpp:318
       // Loop over all of the alias sets in the tracker object.
-      for (AliasSet &AS : *CurAST) {
+      for (auto ASItB = CurAST->begin(), ASItE = CurAST->end();
+           ASItB != ASItE;) {
----------------
It took me a second to see why this was necessary.  This change actually worries me a bit since it implies we may have other cases which need updated after your change.


Repository:
  rL LLVM

https://reviews.llvm.org/D53786