[GISel] Register GISelValueTracking as observer in Combiner/Legalizer to prevent future caching bugs
- Dominant language
- LLVM
- Stars
- 40.5k
- Forks
- 18.7k
- PR merge metrics
- PR metrics pending
Description
I was looking through the GlobalISel code and noticed that `GISelValueTracking` inherits from `GISelChangeObserver`, but we aren't adding it to the observer list in the `Combiner`, `Legalizer`, and other places before modifying instructions.
Since `GISelValueTracking` doesn't cache results right now (even identical calls to `computeKnownBitsImpl` are fully recomputed), this isn't causing bugs today. However, if we ever add caching to `GISelValueTracking` later on, not having it registered as an observer will definitely cause stale-cache bugs.
Would it make sense to add it to the observers now to prevent future issues?
Contributor guide
Research direction
Start by tracing observer registration and instruction-mutation paths in the GlobalISel Combiner and Legalizer, then inspect GISelValueTracking and computeKnownBitsImpl. Done means GISelValueTracking is registered wherever required before modifications, with the existing observer behavior preserved.
Written by the indexing model from the issue text.
Assessment
- Domain
- compilers
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100