[PATCH] D134006: Add an optional cache to computeKnownBits.

Wed Sep 21 01:08:44 PDT 2022

nikic added a comment.

> BasicAA (via MemoryDependenceResults, via GVN) may make many calls to
> computeKnownBits. On my testcase (not able to share, sorry), this cache
> reduces the time spent in computeKnownBits from 2s to 270ms, a 7.5x
> speedup.

MDA has a known problem with overly large recursion cutoffs. I'd be interested to know whether passing `-memdep-block-number-limit=100` would improve compile-time for your case.

================
Comment at: llvm/include/llvm/Analysis/ValueTracking.h:52
+// We can key lookups on the basic block that contains CxtI, rather than on CxtI
+// itself, because the context instr itself is not significant, only its BB.
+using KnownBitsCache = DenseMap<
----------------
jlebar wrote:
> nikic wrote:
> > This is incorrect, the context instruction is used to check applicability of assumes for example, see isValidAssumeForContext().
> `isGuaranteedToTransferExecutionToSuccessor`, wow, TIL.  Thanks.
> 
> But, very sad: With the change to look at the real CtxI rather than its BB, this no longer provides a significant speedup.
> 
> This all seems so silly, because in 99.9% of cases the BB terminator really *is* sufficient.
> 
> Any thoughts?  Or do you think I should give up on this?
I don't have any great ideas on how to do more effective caching without affecting result quality. I guess we could decouple the context-independent from the context-dependent parts somehow, but given that they are currently interleaved, this doesn't sound easy.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134006