[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