[PATCH] D74943: [GISel][KnownBits]{NFC} Add a cache mechanism to speed compile time

[Daniel Sanders via Phabricator via llvm-commits]

dsanders added a comment.

In D74943#1887127 <https://reviews.llvm.org/D74943#1887127>, @qcolombet wrote:

> > That's true (although you'd need a third vreg with no defs/uses to do it) but as you say there's no reason to do so.
>
> That's my point. Even if there is no reason to do so, it is possible.
>  What I am saying is a mechanism that solely relies on registers not changing value is not bug free.


Just because you said 'registers' there I should clarify that it relies on vregs not changing value (which they don't). Physical registers are different matter and those won't be cachable between getKnownBits() calls.

I think the argument that because it's possible to do the wrong thing if you try hard enough that you have to protect against that is rather flawed and inconsistent with the existing code in LLVM. For example, it's possible to create vregs with multiple defs (and quite easy to do by accident) but we don't guard against that in release/debug builds because it's a breach of the SSA contract. We do of course guard against it when the machine verifier is enabled so we find out if somebody violates the contract. That same trust but verify with additional tools approach is appropriate here too. A related issue for that is the ability to serialize analysis passes in MIR so we can run both with a freshly run analysis and maintained data from a previous pass.

If a pass really insists on doing something weird that breaks the contract there will be ways to flush the cache (both fine grain, and the entire thing) just like any other analysis pass but the code breaking the contract will be responsible for repairing the analysis even if it's as simple as telling the pass manager to re-run the analysis (or more accurately not telling the pass manager that the pass preserves it).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74943