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

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 23 14:27:06 PDT 2020


nikic added a comment.

I've now added the additional test.

Regarding the IsCapturedCache, I agree that this is safe to reuse. I thought we might get away with just temporarily swapping out the AliasCache and keeping the rest (https://github.com/llvm/llvm-project/commit/8815a0174d93fb8eadac9c4d82b09edfd1f34319), but it turns out this has worse performance impact (https://llvm-compile-time-tracker.com/compare.php?from=1b65a51af80a9375ef85cb8fa6ec9ec3c68b3549&to=8815a0174d93fb8eadac9c4d82b09edfd1f34319&stat=instructions). Probably because swapping out a SmallDenseSet of MemoryLocation pairs is expensive -- these are large structures.

For efficient reuse we'd want to convert the capture cache into a pointer that either points into an owned cache in AAQueryInfo, or into a separate cache. However, that introduces a self-reference, so we'd also have to deal with custom move/copy constructors/assignments. Given that the impact doesn't seem so large, I'm not sure it's worth the bother.


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

https://reviews.llvm.org/D90066



More information about the llvm-commits mailing list