I generally think that we should be moving away from automatically updated value handles (or listeners), in favor of explicitly invalidating analyses when instructions are erased.
However, a big problem is that we currently don’t actually have a good way to ensure that all the necessary invalidation is performed. We have two mechanisms for this:
- AssertingVH: Asserts immediately if value is erased without being dropped from the analysis first. This is great – but is fundamentally incompatible with analyses managed by the new pass manager. If a pass does not preserve an analyses, it will still remain live throughout the execution of the pass, and only get invalidated when the pass does not mark it as preserved. This means that AssertingVH can only be used for data structures that do not outlive a pass.
- PoisoningVH: The NewPM-compatible alternative are poisoning value handles, which mark the value as poisoned on erase and then assert only when it’s accessed later. This gives worse diagnostics (because the assert happens at point of use rather than at point of erase, where the bug actually is). But much more importantly, it will only assert if the PoisoningVH is actually accessed again – which for the typical use as a DenseMap key, will only happen if actual address reuse happens. This means that basic test coverage run in assertion-enabled builds is likely not sufficient to identify issues.
I’d love to change some analyses like LVI to not use value handles, as doing explicit invalidation should not be a major imposition for users of the analysis — however, I’m not comfortable doing that without some assurance that all the necessary invalidation takes places.
If anyone has ideas on how we can reliably detect missing invalidation in a NewPM-compatible way, I’m all ears…