[llvm] [AssumptionCache] Remove incorrect assertion from `removeAffectedValues()` (PR #214524)
Thurston Dang via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 12 09:50:47 PDT 2026
thurstond wrote:
> > Dumb question: is it infeasible to add notifications to Use::set(),
>
> I had a think about this and decided against basically because we'd require a new type of value handle along with all of the associated plumbing and pay with a non-trivial compile time overhead in `Use::set()` only to implement a per-user observer pattern with no precedent in the rest of the codebase. This didn't seem like a good trade.
Agreed, not a good trade.
> > or fix the pass (loop-rotate in this case)?
>
> `loop-rotate` is not incorrect in what it's doing. The assumption cache is supposed to be self-updating and conservative. If anything is at fault, it's the assumption cache.
The comment for the `verifyAnalysis()` function is open to the possibility that AssumptionCache may need cooperation from other passes:
https://github.com/llvm/llvm-project/blob/bf6872fd03c9096600b7b47251be223e40b38124/llvm/lib/Analysis/AssumptionCache.cpp#L346-L350
> @thurstond Let me know if you find the above convincing enough! If so, I'll land this today, otherwise we can discuss designs.
As is, the assertion is obviously wrong, and I don't want to delay your fix by bikeshedding, so please feel free to land it. Thanks for the fix! :-)
https://github.com/llvm/llvm-project/pull/214524
More information about the llvm-commits
mailing list