[PATCH] D59151: [BasicAA] Reduce no of map searches. Assert map entry exists. [NFCI]

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

LGTM, really nice!



================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:1606-1608
+        auto Pair = AliasCache.insert(std::make_pair(Locs, OrigAliasResult));
+        assert(!Pair.second && "Entry must have existed");
+        Pair.first->second = OrigAliasResult;
----------------
asbirlea wrote:
> chandlerc wrote:
> > Here and below, the original code does the same number of lookups and seems more clear?
> Yes, that's true. This change refers to the second bullet point in the summary: adding the assert to check an entry was already in the map and it should only be updated here, never created.
> I got tripped by this by accidentally clearing the cache and going into an infinite recursion.
> 
> Let me know if you'd prefer this in a separate change or the title updated to include this.
No, I'd missed that this was enabling the assert, and I understand why that's valuable here, thanks.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59151