[PATCH] D149529: [SCEV][reland] More precise trip multiples

[Joshua Cao via Phabricator via llvm-commits]

caojoshua added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:14326
+         (Multiple == 0 || RecomputedMultiple == 0)) &&
+        RecomputedMultiple.urem(Multiple) != 0) {
+      dbgs() << "Incorrect cached computation in ConstantMultipleCache for "
----------------
uabelho wrote:
> It's this urem call that crashes in the comment I made yesterday.
> 
> Multiple is 0 and doing urem with RHS being 0 hits the assertion since dividing by  0 isn't good.
> 
> Are we perhaps missing a negation of the condition
> ```
> (Multiple == 0 || RecomputedMultiple == 0)
> ```
> ?
> Now we do the urem(Multiple) specifically if Multiple is 0, which we should avoid.
Its due to returning a zero from [too many trailing zeros](https://github.com/llvm/llvm-project/blob/b7e5cb1f9a3a5226f22bb81c865214be81dce940/llvm/lib/Analysis/ScalarEvolution.cpp#L6233). I am going to rewrite this a bit, it should be verifying on `getNonZeroConstantMultiple()`.

I am going to run this through expensive checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149529