[llvm] [SCEVExpander] Support hoisting udiv X, Y where Y is non-zero (PR #96102)

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 1 09:23:04 PDT 2024


================
@@ -1470,12 +1470,11 @@ Value *SCEVExpander::expand(const SCEV *S) {
 
   // We can move insertion point only if there is no div or rem operations
   // otherwise we are risky to move it over the check for zero denominator.
-  auto SafeToHoist = [](const SCEV *S) {
-    return !SCEVExprContains(S, [](const SCEV *S) {
+  auto SafeToHoist = [&](const SCEV *S) {
+    return !SCEVExprContains(S, [&](const SCEV *S) {
               if (const auto *D = dyn_cast<SCEVUDivExpr>(S)) {
-                if (const auto *SC = dyn_cast<SCEVConstant>(D->getRHS()))
-                  // Division by non-zero constants can be hoisted.
-                  return SC->getValue()->isZero();
+                if (SE.isKnownNonZero(D->getRHS()))
----------------
preames wrote:

Every place we consider hoisting a udiv appears to be wrong by this logic.  We also appear to need to worry about undef by the same logic.  We have several different instances in SCEV Expander alone.  

We also have no infrastructure for proving a SCEV non-poison directly, so the scope of a fix here is significant.  I think we'd basically have to duplicate all of isGuaranteedNotToBeUndefOrPoison over SCEV.

I'm not going to drive this forward as this patch was merely a small fix observed while exploring something else, and the effort doesn't appear justified  

https://github.com/llvm/llvm-project/pull/96102


More information about the llvm-commits mailing list