[PATCH] D141823: [SCEV] More precise trip multiples

Joshua Cao via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 21 22:57:14 PDT 2023


caojoshua added a comment.

> Whenever a new cache is introduced, it is highly recommended to add logic to ScalarEvoltion::verify for it to make sure it is sane. Can you please add that?

I'm not sure what it means to be sane in this case. It makes sense for certain caches. For example, I see that `verify()` checks that Values that exist in ExprValueMap also exist in the reversed ValueExprMap. From what I can see, the current validations all expect some certain values to currently be in a cache, but this does not apply to ConstantMultipleCache.

For now, I added verification that the cached value is aligned with `getConstantMultipleImpl()`. It looks like the first verification of this kind. I'm not sure how useful it is, because `getConstantMultipleImpl()` makes calls to `getConstantMultiple()`, which retrieves from the cache. I also added `-passes=verify<scalar-evolution>` to two tests, since there are actually very few tests that do run verification.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141823



More information about the llvm-commits mailing list