[PATCH] D142330: [AssumptionCache] caches @llvm.experimental.guard's

Joshua Cao via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Feb 25 14:12:25 PST 2023


caojoshua added a comment.

> If this change goes forward, we need to investigate all places where assumes/AssumptionCache is used and see if what is being done for assumes holds for guards as well. In LVI, what was done for assumes did not hold for guards.
> Also, in future, when anyone adds assumption cache (under the idea that it handles assumes), we can very easily fall into the problem of having a miscompile with guards. For that reason and given that we are working towards moving away from this implementation, I would suggest we abandon this 
> change altogether.

I agree with Anna here. Although it should be possible to make this change work with `isValidAssumeForContext()`, it will be a lot of work and testing a bunch of passes with little benefit. And we hope to move away from guards anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142330



More information about the llvm-commits mailing list