[PATCH] D57627: [BasicAA] Cache nonEscapingLocalObjects for alias() calls.

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added inline comments.


================
Comment at: lib/Analysis/BasicAliasAnalysis.cpp:122-126
+  if (IsCapturedCache) {
+    auto CacheIt = IsCapturedCache->find(V);
+    if (CacheIt != IsCapturedCache->end())
+      return CacheIt->second;
+  }
----------------
chandlerc wrote:
> asbirlea wrote:
> > chandlerc wrote:
> > > Can this function ever be re-entered? It seems unlikely given the structure.
> > > 
> > > If not, hoist the `CacheIt` iterator out, use insert, and then update it below rather than doing another lookup?
> > IIUC, insert doesn't do a lookup. So using insert below (instead of `[]`) should be equivalent to your suggestion? We are guaranteed we need to insert if we reached that point..
> Er, insert does do a lookup?
> 
> ```
>   auto CacheIt;
>   if (IsCapturedCache) {
>     bool Inserted;
>     std::tie(CacheIt, Inserted) = IsCapturedCache->insert({V, false});
>     if (!Inserted)
>       // Found cached result, return it!
>       return CacheIt->second;
>   }
>   ...
>   bool Ret = !PointerMayBeCaptured(...);
>   if (IsCapturedCache)
>     CacheIt->second = Ret;
>   return Ret;
> ```
> 
> This kind of code pattern should do a single lookup into the map instead of doing two.
Ah, you're right! Not sure why I was thinking that if *I* know the value is not in the set, the *set* will magically know too...

Thanks, updated.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57627