[PATCH] D95521: [SCEV] Apply loop guards to trailing zero bits

[Gil Rapaport via Phabricator via llvm-commits]

gilr marked 3 inline comments as done.
gilr added inline comments.


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:13266
+      // zero %v's D low bits.
+      if (auto ZExt = dyn_cast<SCEVZeroExtendExpr>(LHS)) {
+        if (auto Trunc = dyn_cast<SCEVTruncateExpr>(ZExt->getOperand(0)))
----------------
fhahn wrote:
> Here we basically are matching a SCEV `URem` expression, right? There's a helper for that (`matchURem`), which should help to simplify the code a bit? 
Ah, indeed. Thanks!


================
Comment at: llvm/lib/Analysis/ScalarEvolution.cpp:13277
+              auto ClearLowBits = getMulExpr(
+                  getUDivExpr(LHSUnknown, Divisor), Divisor,
+                  (SCEV::NoWrapFlags)(SCEV::FlagNUW | SCEV::FlagNSW));
----------------
fhahn wrote:
> Here we could just re-use the existing `URem` expression?
Do you mean by computing `(A / B) * B` from the URem expression e.g. via `A - URemExpr`?


================
Comment at: llvm/test/Transforms/LoopUnroll/runtime-unroll-assume-no-remainder.ll:45
+entry:
+  br i1 %c, label %bail.out, label %do.work
+
----------------
fhahn wrote:
> is this extra branch needed?
This was done in D94088 to prevent D93974 (should it ever get in) from validating the entry-block assume for `GetMinTrailingZeros()` w/o an assumption context. But since `appyLoopGuards()` uses its own assumption validation check it's indeed redundant. Will remove.


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

https://reviews.llvm.org/D95521